From df1995935773c9963573448cc5d02a66de8b215b Mon Sep 17 00:00:00 2001 From: Vitalii Solovei Date: Tue, 6 Oct 2026 11:49:50 +0200 Subject: [PATCH 01/14] make UserTranslationRequest.profileId a required field (WP-1022) First step of letting a user choose which Smartling profile/project a translation request goes to, instead of relying on whichever profile is flagged "active": the request model itself now requires the caller to say which profile it's for, validated the same way the existing required job/source fields are. Updated every constructor/fromArray() call site (TestRunController, and the test suites exercising UserTranslationRequest/ ContentRelationsDiscoveryService/ContentRelationsHandler) to pass one. Note: ContentRelationsDiscoveryService::createSubmissions() doesn't read getProfileId() yet - it still resolves the active profile unconditionally. That wiring, plus the frontend profile selector that will actually populate this field in real AJAX requests, are separate steps in the same ticket. --- inc/Smartling/Models/UserTranslationRequest.php | 13 ++++++++++++- inc/Smartling/WP/Controller/TestRunController.php | 1 + .../IntegrationTests/tests/SubmissionUploadTest.php | 2 ++ tests/Models/TranslationRequestTest.php | 12 ++++++++++++ .../ContentRelationsDiscoveryServiceTest.php | 7 +++++++ tests/Services/ContentRelationsHandlerTest.php | 1 + 6 files changed, 35 insertions(+), 1 deletion(-) diff --git a/inc/Smartling/Models/UserTranslationRequest.php b/inc/Smartling/Models/UserTranslationRequest.php index a2a2991b5..2c644b502 100644 --- a/inc/Smartling/Models/UserTranslationRequest.php +++ b/inc/Smartling/Models/UserTranslationRequest.php @@ -13,9 +13,10 @@ class UserTranslationRequest private array $relations; private array $targetBlogIds; private JobInformation $jobInformation; + private int $profileId; private array $ids; - public function __construct(int $contentId, string $contentType, array $relations, array $targetBlogIds, JobInformation $jobInformation, array $ids = [], string $description = '') + public function __construct(int $contentId, string $contentType, array $relations, array $targetBlogIds, JobInformation $jobInformation, int $profileId, array $ids = [], string $description = '') { $this->contentId = $contentId; $this->contentType = $contentType; @@ -24,6 +25,7 @@ public function __construct(int $contentId, string $contentType, array $relation $this->relations = $relations; $this->targetBlogIds = ArrayHelper::toArrayOfIntegers($targetBlogIds, 'Target blog id expected to be numeric'); $this->jobInformation = $jobInformation; + $this->profileId = $profileId; $this->ids = self::toIntegerArray($ids); } @@ -60,6 +62,11 @@ public function getJobInformation(): JobInformation return $this->jobInformation; } + public function getProfileId(): int + { + return $this->profileId; + } + public function getIds(): array { return $this->ids; @@ -77,6 +84,7 @@ public static function fromArray(array $array): self $array['relations'] ?? [], explode(',', $array['targetBlogIds']), new JobInformation($array['job']['id'], $array['job']['authorize'] === 'true', $array['job']['name'], $array['job']['description'], $array['job']['dueDate'], $array['job']['timeZone']), + (int)$array['profileId'], $ids, $array['description'] ?? (count($ids) > 0 ? 'From Bulk Submit' : 'From Widget'), ); @@ -122,6 +130,9 @@ private static function validate(array $array): void if (!array_key_exists('timeZone', $array['job'])) { throw new \InvalidArgumentException('Job time zone required'); } + if (!array_key_exists('profileId', $array)) { + throw new \InvalidArgumentException('Profile id required'); + } } private static function toIntegerArray(array $ids): array diff --git a/inc/Smartling/WP/Controller/TestRunController.php b/inc/Smartling/WP/Controller/TestRunController.php index 916b6e25c..36e10e078 100644 --- a/inc/Smartling/WP/Controller/TestRunController.php +++ b/inc/Smartling/WP/Controller/TestRunController.php @@ -199,6 +199,7 @@ public function testRun($data): void ->getRelations($post->post_type, $post->ID, [$targetBlogId])->getReferences()], [$targetBlogId], new JobInformation($job->getJobUid(), true, $job->getJobName(), 'Test run job', '', 'UTC'), + $profile->getId(), [], 'Test run' )); diff --git a/tests/IntegrationTests/tests/SubmissionUploadTest.php b/tests/IntegrationTests/tests/SubmissionUploadTest.php index e503ea1e4..04c9dd941 100644 --- a/tests/IntegrationTests/tests/SubmissionUploadTest.php +++ b/tests/IntegrationTests/tests/SubmissionUploadTest.php @@ -39,6 +39,7 @@ public function testUploadMultipleTargets() [], $targetBlogs, new JobInformation($job['translationJobUid'], false, $jobName, '', '', ''), + $profile->getId(), )); $submissions = $submissionManager->find([SubmissionEntity::FIELD_SOURCE_ID => $postId]); $this->assertCount(2, $submissions, 'Expected two new submissions to be created'); @@ -88,6 +89,7 @@ public function testUploadAttachment() [2 => ['attachment' => [$attachmentId]]], $targetBlogs, new JobInformation($job['translationJobUid'], false, $jobName, '', '', ''), + $profile->getId(), )); $this->assertCount($existingSubmissionCount + 2, $submissionManager->find([1 => 1])); // findOne returns null on multiple submissions diff --git a/tests/Models/TranslationRequestTest.php b/tests/Models/TranslationRequestTest.php index 6084683a9..f76c4cd83 100644 --- a/tests/Models/TranslationRequestTest.php +++ b/tests/Models/TranslationRequestTest.php @@ -35,6 +35,7 @@ public function testFromArray() 2 => [$targetBlogId => ['attachment' => [5]]], ], 'targetBlogIds' => (string)$targetBlogId, + 'profileId' => 9, ]); $this->assertEquals($sourceId, $x->getContentId()); $this->assertEquals($sourceContentType, $x->getContentType()); @@ -45,6 +46,15 @@ public function testFromArray() $this->assertEquals($jobName, $x->getJobInformation()->getName()); $this->assertEquals($jobTimeZone, $x->getJobInformation()->getTimeZone()); $this->assertEquals($jobUid, $x->getJobInformation()->getId()); + $this->assertEquals(9, $x->getProfileId()); + } + + public function testFromArrayRequiresProfileId() + { + $this->expectException(\InvalidArgumentException::class); + $array = $this->buildArray(); + unset($array['profileId']); + UserTranslationRequest::fromArray($array); } public function testFromArrayBulkUploadWithEmptySourceId() @@ -65,6 +75,7 @@ public function testFromArrayBulkUploadWithEmptySourceId() 'relations' => [], 'targetBlogIds' => (string)$targetBlogId, 'ids' => $ids, + 'profileId' => 9, ]); $this->assertTrue($x->isBulk()); $this->assertEquals($ids, $x->getIds()); @@ -104,6 +115,7 @@ private function buildArray(array $overrides = []): array 'source' => ['id' => [5], 'contentType' => 'post'], 'relations' => [], 'targetBlogIds' => '2', + 'profileId' => 9, ], $overrides); } } diff --git a/tests/Services/ContentRelationsDiscoveryServiceTest.php b/tests/Services/ContentRelationsDiscoveryServiceTest.php index ca8b89905..fd5f7fe27 100644 --- a/tests/Services/ContentRelationsDiscoveryServiceTest.php +++ b/tests/Services/ContentRelationsDiscoveryServiceTest.php @@ -151,6 +151,7 @@ public function testCreateSubmissionsHandler() ], 'targetBlogIds' => $targetBlogId, 'relations' => [], + 'profileId' => 5, ])); } @@ -210,6 +211,7 @@ public function testCreateSubmissionsHandlerStampsConfigurationProfileOnExisting ], 'targetBlogIds' => $targetBlogId, 'relations' => [], + 'profileId' => 5, ])); } @@ -272,6 +274,7 @@ function (ConfigurationProfileEntity $profile, string $jobUid, array $fileUris): ], 'targetBlogIds' => $targetBlogId, 'relations' => [$targetBlogId => ['post' => [17], 'attachment' => [23]]], + 'profileId' => 5, ])); } public function testBulkSubmitHandler() @@ -361,6 +364,7 @@ public function testBulkSubmitHandler() ], 'targetBlogIds' => $targetBlogId, 'ids' => $sourceIds, + 'profileId' => 5, ])); } @@ -427,6 +431,7 @@ public function testBulkSubmitHandlerStampsConfigurationProfile() ], 'targetBlogIds' => $targetBlogId, 'ids' => $sourceIds, + 'profileId' => 5, ])); } @@ -613,6 +618,7 @@ public function testJobInfoGetsStoredOnNewSubmissions() ], 'targetBlogIds' => $targetBlogId, 'relations' => [], + 'profileId' => 5, ])); $this->restoreDependencyInjection(); } @@ -804,6 +810,7 @@ public function testRelatedItemsSentForTranslation() 'source' => ['id' => [$sourceId], 'contentType' => $contentType], 'relations' => [$targetBlogId => ['post' => [$depth1AttachmentId], 'attachment' => [$depth2AttachmentId]]], 'targetBlogIds' => (string)$targetBlogId, + 'profileId' => 5, ])); if ($this->exception !== null) { throw $this->exception; diff --git a/tests/Services/ContentRelationsHandlerTest.php b/tests/Services/ContentRelationsHandlerTest.php index f75a2a779..4ace2bdac 100644 --- a/tests/Services/ContentRelationsHandlerTest.php +++ b/tests/Services/ContentRelationsHandlerTest.php @@ -115,6 +115,7 @@ private function buildData(array $overrides = []): array 'timeZone' => 'Europe/Kyiv', 'authorize' => 'true', ], + 'profileId' => 5, ], $overrides); } } From 36084e4422e36b91c9753969644af15db7aa19db Mon Sep 17 00:00:00 2001 From: Vitalii Solovei Date: Tue, 6 Oct 2026 11:55:10 +0200 Subject: [PATCH 02/14] resolve the user-requested profile in createSubmissions() (WP-1022) createSubmissions() always resolved the profile via getSingleSettingsProfile($curBlogId) - whichever one happens to be flagged active - ignoring UserTranslationRequest::getProfileId() entirely. Now resolves the requested profile explicitly (validating it exists, belongs to the current blog, and is active), falling back to today's active-profile behavior only if the requested id is stale (profile deactivated/deleted client-side after the page loaded) rather than hard-failing the whole request. This is the single resolution point both the bulk and non-bulk branches already share, so the profile-stamping added for WP-1021 (createBatch(), setConfigurationProfileId() at enqueue time) now automatically uses the user's explicit choice with no further changes needed at those call sites. --- .../ContentRelationsDiscoveryService.php | 27 +++- .../ContentRelationsDiscoveryServiceTest.php | 147 ++++++++++++++++++ 2 files changed, 173 insertions(+), 1 deletion(-) diff --git a/inc/Smartling/Services/ContentRelationsDiscoveryService.php b/inc/Smartling/Services/ContentRelationsDiscoveryService.php index 07ca71fc1..bc3cafe48 100644 --- a/inc/Smartling/Services/ContentRelationsDiscoveryService.php +++ b/inc/Smartling/Services/ContentRelationsDiscoveryService.php @@ -13,6 +13,7 @@ use Smartling\DbAl\UploadQueueManager; use Smartling\DbAl\WordpressContentEntities\EntityWithMetadata; use Smartling\Exception\EntityNotFoundException; +use Smartling\Exception\SmartlingDbException; use Smartling\Exception\SmartlingGutenbergParserNotFoundException; use Smartling\Exception\SmartlingHumanReadableException; use Smartling\Extensions\Acf\AcfDynamicSupport; @@ -150,10 +151,34 @@ public function bulkUpload( return $queueIds; } + /** + * Resolves the profile the user explicitly chose for this request, rather than whichever + * one happens to be flagged active. Falls back to today's active-profile behavior if the + * requested id doesn't exist, belongs to a different blog, or isn't active - this can + * happen with stale client-side data (profile deactivated/deleted after the page loaded) + * and shouldn't hard-fail the whole request. + * + * @throws SmartlingDbException + */ + private function resolveRequestedProfile(int $requestedProfileId, int $curBlogId): ConfigurationProfileEntity + { + $profile = ArrayHelper::first($this->settingsManager->getEntityById($requestedProfileId)); + if ($profile instanceof ConfigurationProfileEntity) { + if ($profile->getSourceLocale()->getBlogId() === $curBlogId && 1 === $profile->getIsActive()) { + return $profile; + } + $this->getLogger()->warning("Requested profileId=$requestedProfileId is not an active profile for blogId=$curBlogId, falling back to active profile"); + } else { + $this->getLogger()->warning("Requested profileId=$requestedProfileId not found, falling back to active profile"); + } + + return $this->settingsManager->getSingleSettingsProfile($curBlogId); + } + public function createSubmissions(UserTranslationRequest $request): void { $curBlogId = $this->wordpressProxy->get_current_blog_id(); - $profile = $this->settingsManager->getSingleSettingsProfile($curBlogId); + $profile = $this->resolveRequestedProfile($request->getProfileId(), $curBlogId); $job = $request->getJobInformation(); $jobInfo = new JobEntity($job->getName(), $job->getId(), $profile->getProjectId()); diff --git a/tests/Services/ContentRelationsDiscoveryServiceTest.php b/tests/Services/ContentRelationsDiscoveryServiceTest.php index fd5f7fe27..838317f92 100644 --- a/tests/Services/ContentRelationsDiscoveryServiceTest.php +++ b/tests/Services/ContentRelationsDiscoveryServiceTest.php @@ -59,6 +59,7 @@ function apply_filters($a, ...$b) { use Smartling\Services\ContentRelationsDiscoveryService; use Smartling\Services\ContentRelationsHandler; use Smartling\Settings\ConfigurationProfileEntity; + use Smartling\Settings\Locale; use Smartling\Settings\SettingsManager; use Smartling\Submissions\SubmissionEntity; use Smartling\Submissions\SubmissionFactory; @@ -215,6 +216,152 @@ public function testCreateSubmissionsHandlerStampsConfigurationProfileOnExisting ])); } + /** + * @dataProvider invalidRequestedProfileProvider + */ + public function testCreateSubmissionsFallsBackToActiveProfileWhenRequestedOneIsNotUsable(?ConfigurationProfileEntity $requestedProfile) + { + $sourceBlogId = 1; + $sourceId = 48; + $contentType = 'post'; + $targetBlogId = 2; + $jobName = 'Job Name'; + $jobUid = 'abcdef123456'; + $requestedProfileId = 5; + + $activeProfile = $this->createMock(ConfigurationProfileEntity::class); + $activeProfile->method('getProjectId')->willReturn('activeProjectUid'); + + $apiWrapper = $this->createMock(ApiWrapper::class); + $apiWrapper->expects($this->once())->method('createAuditLogRecord')->willReturnCallback( + function (ConfigurationProfileEntity $configurationProfile) use ($activeProfile): void { + $this->assertSame($activeProfile, $configurationProfile, 'Must fall back to the active profile, not the unusable requested one'); + }, + ); + + $settingsManager = $this->createMock(SettingsManager::class); + $settingsManager->method('getEntityById')->with($requestedProfileId)->willReturn($requestedProfile === null ? [] : [$requestedProfile]); + $settingsManager->expects(self::once())->method('getSingleSettingsProfile')->with($sourceBlogId)->willReturn($activeProfile); + + $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); + + $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' => [], + 'profileId' => $requestedProfileId, + ])); + } + + public static function invalidRequestedProfileProvider(): array + { + $foreignBlogLocale = new Locale(); + $foreignBlogLocale->setBlogId(99); // not the current blog (1) + $foreignBlogProfile = new ConfigurationProfileEntity(); + $foreignBlogProfile->setSourceLocale($foreignBlogLocale); + $foreignBlogProfile->setIsActive(1); + + $sameBlogLocale = new Locale(); + $sameBlogLocale->setBlogId(1); + $inactiveProfile = new ConfigurationProfileEntity(); + $inactiveProfile->setSourceLocale($sameBlogLocale); + $inactiveProfile->setIsActive(0); + + return [ + 'profile does not exist' => [null], + 'profile belongs to a different blog' => [$foreignBlogProfile], + 'profile is not active' => [$inactiveProfile], + ]; + } + + public function testCreateSubmissionsUsesRequestedProfileWhenValidAndActive() + { + $sourceBlogId = 1; + $sourceId = 48; + $contentType = 'post'; + $targetBlogId = 2; + $jobName = 'Job Name'; + $jobUid = 'abcdef123456'; + $requestedProfileId = 5; + + $sourceLocale = new Locale(); + $sourceLocale->setBlogId($sourceBlogId); + $requestedProfile = $this->createMock(ConfigurationProfileEntity::class); + $requestedProfile->method('getSourceLocale')->willReturn($sourceLocale); + $requestedProfile->method('getIsActive')->willReturn(1); + $requestedProfile->method('getProjectId')->willReturn('requestedProjectUid'); + + $apiWrapper = $this->createMock(ApiWrapper::class); + $apiWrapper->expects($this->once())->method('createAuditLogRecord')->willReturnCallback( + function (ConfigurationProfileEntity $configurationProfile) use ($requestedProfile): void { + $this->assertSame($requestedProfile, $configurationProfile); + }, + ); + + $settingsManager = $this->createMock(SettingsManager::class); + $settingsManager->method('getEntityById')->with($requestedProfileId)->willReturn([$requestedProfile]); + $settingsManager->expects(self::never())->method('getSingleSettingsProfile'); + + $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); + + $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' => [], + 'profileId' => $requestedProfileId, + ])); + } + public function testCreateSubmissionsRelations() { $sourceBlogId = 1; From 512a55a338c5d3f7961b0da76c70f9638ec0f87f Mon Sep 17 00:00:00 2001 From: Vitalii Solovei Date: Tue, 6 Oct 2026 12:12:03 +0200 Subject: [PATCH 03/14] honor user-requested profile in smartling_job_api_proxy (WP-1022) list-jobs/create-job previously always resolved the blog's active profile via getSingleSettingsProfile(), ignoring any profileId the client may have selected. Mirror the fallback behavior already added to ContentRelationsDiscoveryService: use the requested profile when it exists, belongs to the current blog, and is active; otherwise fall back to the active profile. Co-Authored-By: Claude Sonnet 5 --- .../Controller/ContentEditJobController.php | 31 +++- .../ContentEditJobControllerTest.php | 141 ++++++++++++++++++ 2 files changed, 171 insertions(+), 1 deletion(-) create mode 100644 tests/Smartling/WP/Controller/ContentEditJobControllerTest.php diff --git a/inc/Smartling/WP/Controller/ContentEditJobController.php b/inc/Smartling/WP/Controller/ContentEditJobController.php index 0bdb2737b..fc99e697d 100644 --- a/inc/Smartling/WP/Controller/ContentEditJobController.php +++ b/inc/Smartling/WP/Controller/ContentEditJobController.php @@ -7,6 +7,7 @@ use Smartling\ApiWrapperInterface; use Smartling\Bootstrap; use Smartling\DbAl\LocalizationPluginProxyInterface; +use Smartling\Exception\SmartlingDbException; use Smartling\Exceptions\SmartlingApiException; use Smartling\Helpers\ArrayHelper; use Smartling\Helpers\Cache; @@ -17,6 +18,7 @@ use Smartling\Helpers\SiteHelper; use Smartling\Helpers\SmartlingUserCapabilities; use Smartling\Helpers\WordpressFunctionProxyHelper; +use Smartling\Settings\ConfigurationProfileEntity; use Smartling\Settings\SettingsManager; use Smartling\Submissions\SubmissionManager; use Smartling\Vendor\Smartling\Jobs\JobStatus; @@ -81,6 +83,32 @@ public function setServedContentType($servedContentType) $this->servedContentType = $servedContentType; } + /** + * Resolves the profile the user explicitly chose (e.g. from a profile dropdown), rather + * than whichever one happens to be flagged active. Falls back to today's active-profile + * behavior if no profile was requested, or the requested id doesn't exist, belongs to a + * different blog, or isn't active - this can happen with stale client-side data (profile + * deactivated/deleted after the page loaded) and shouldn't hard-fail the request. + * + * @throws SmartlingDbException + */ + private function resolveRequestedProfile(?int $requestedProfileId, int $curBlogId): ConfigurationProfileEntity + { + if ($requestedProfileId !== null) { + $profile = ArrayHelper::first($this->settingsManager->getEntityById($requestedProfileId)); + if ($profile instanceof ConfigurationProfileEntity) { + if ($profile->getSourceLocale()->getBlogId() === $curBlogId && 1 === $profile->getIsActive()) { + return $profile; + } + $this->getLogger()->warning("Requested profileId=$requestedProfileId is not an active profile for blogId=$curBlogId, falling back to active profile"); + } else { + $this->getLogger()->warning("Requested profileId=$requestedProfileId not found, falling back to active profile"); + } + } + + return $this->settingsManager->getSingleSettingsProfile($curBlogId); + } + public function initJobApiProxy(): void { add_action('wp_ajax_' . self::SMARTLING_JOB_API_PROXY, function () { @@ -101,8 +129,9 @@ public function initJobApiProxy(): void 'status' => 200, ]; - $profile = $this->settingsManager->getSingleSettingsProfile($this->siteHelper->getCurrentBlogId()); $params = &$data['params']; + $requestedProfileId = isset($params['profileId']) ? (int)$params['profileId'] : null; + $profile = $this->resolveRequestedProfile($requestedProfileId, $this->siteHelper->getCurrentBlogId()); $validateRequires = static function ($fieldName) use (&$result, $params) { $value = trim($params[$fieldName] ?? ''); diff --git a/tests/Smartling/WP/Controller/ContentEditJobControllerTest.php b/tests/Smartling/WP/Controller/ContentEditJobControllerTest.php new file mode 100644 index 000000000..f46a593b5 --- /dev/null +++ b/tests/Smartling/WP/Controller/ContentEditJobControllerTest.php @@ -0,0 +1,141 @@ +createMock(ApiWrapperInterface::class), + $this->createMock(LocalizationPluginProxyInterface::class), + $this->createMock(PluginInfo::class), + $settingsManager, + $this->createMock(SiteHelper::class), + $this->getMockBuilder(SubmissionManager::class)->disableOriginalConstructor()->getMock(), + $this->createMock(Cache::class), + $this->createMock(WordpressFunctionProxyHelper::class), + ); + } + + private function profileWithId(int $id): ConfigurationProfileEntity + { + $profile = new ConfigurationProfileEntity(); + $profile->setId($id); + + return $profile; + } + + public function testResolveRequestedProfileUsesRequestedWhenValidAndActive() + { + $blogId = 1; + $requested = $this->profileWithId(5); + $sourceLocale = new Locale(); + $sourceLocale->setBlogId($blogId); + $requested->setSourceLocale($sourceLocale); + $requested->setIsActive(1); + + $settingsManager = $this->createMock(SettingsManager::class); + $settingsManager->expects(self::once())->method('getEntityById')->with(5)->willReturn([$requested]); + $settingsManager->expects(self::never())->method('getSingleSettingsProfile'); + + $result = $this->invokeMethod($this->getController($settingsManager), 'resolveRequestedProfile', [5, $blogId]); + + self::assertSame($requested, $result); + } + + public function testResolveRequestedProfileFallsBackWhenNoneRequested() + { + $blogId = 1; + $active = $this->profileWithId(3); + + $settingsManager = $this->createMock(SettingsManager::class); + $settingsManager->expects(self::never())->method('getEntityById'); + $settingsManager->expects(self::once())->method('getSingleSettingsProfile')->with($blogId)->willReturn($active); + + $result = $this->invokeMethod($this->getController($settingsManager), 'resolveRequestedProfile', [null, $blogId]); + + self::assertSame($active, $result); + } + + public function testResolveRequestedProfileFallsBackWhenRequestedNotFound() + { + $blogId = 1; + $active = $this->profileWithId(3); + + $settingsManager = $this->createMock(SettingsManager::class); + $settingsManager->method('getEntityById')->with(5)->willReturn([]); + $settingsManager->expects(self::once())->method('getSingleSettingsProfile')->with($blogId)->willReturn($active); + + $result = $this->invokeMethod($this->getController($settingsManager), 'resolveRequestedProfile', [5, $blogId]); + + self::assertSame($active, $result); + } + + public function testResolveRequestedProfileFallsBackWhenRequestedBelongsToDifferentBlog() + { + $blogId = 1; + $requested = $this->profileWithId(5); + $foreignLocale = new Locale(); + $foreignLocale->setBlogId(99); + $requested->setSourceLocale($foreignLocale); + $requested->setIsActive(1); + $active = $this->profileWithId(3); + + $settingsManager = $this->createMock(SettingsManager::class); + $settingsManager->method('getEntityById')->with(5)->willReturn([$requested]); + $settingsManager->expects(self::once())->method('getSingleSettingsProfile')->with($blogId)->willReturn($active); + + $result = $this->invokeMethod($this->getController($settingsManager), 'resolveRequestedProfile', [5, $blogId]); + + self::assertSame($active, $result); + } + + public function testResolveRequestedProfileFallsBackWhenRequestedIsInactive() + { + $blogId = 1; + $requested = $this->profileWithId(5); + $sourceLocale = new Locale(); + $sourceLocale->setBlogId($blogId); + $requested->setSourceLocale($sourceLocale); + $requested->setIsActive(0); + $active = $this->profileWithId(3); + + $settingsManager = $this->createMock(SettingsManager::class); + $settingsManager->method('getEntityById')->with(5)->willReturn([$requested]); + $settingsManager->expects(self::once())->method('getSingleSettingsProfile')->with($blogId)->willReturn($active); + + $result = $this->invokeMethod($this->getController($settingsManager), 'resolveRequestedProfile', [5, $blogId]); + + self::assertSame($active, $result); + } + + public function testResolveRequestedProfilePropagatesExceptionWhenNoActiveProfileEither() + { + $this->expectException(SmartlingDbException::class); + $blogId = 1; + + $settingsManager = $this->createMock(SettingsManager::class); + $settingsManager->method('getEntityById')->with(5)->willReturn([]); + $settingsManager->method('getSingleSettingsProfile')->with($blogId)->willThrowException(new SmartlingDbException('no active profile')); + + $this->invokeMethod($this->getController($settingsManager), 'resolveRequestedProfile', [5, $blogId]); + } +} From 0e82d572fcc298f6adc877ac383eaf5d4ae074aa Mon Sep 17 00:00:00 2001 From: Vitalii Solovei Date: Tue, 6 Oct 2026 12:19:32 +0200 Subject: [PATCH 04/14] let users pick the translation profile in the job wizard UI (WP-1022) Surface all active profiles for the current blog (not just the first one) to the React job wizard shared by the post/taxonomy edit screen and bulk submit page, each with its own target-locale set. Add a profile dropdown (shown only when more than one profile applies), remember the chosen profile per-blog in localStorage, and send its id with list-jobs/create-job/smartling-create-submissions so the request is routed to the profile the user picked instead of whichever one happens to be flagged active. Switching profiles reconciles the selected target locales and refreshes the existing-jobs list. Co-Authored-By: Claude Sonnet 5 --- .../WP/Controller/BulkSubmitController.php | 1 + .../Controller/ContentEditJobController.php | 2 + .../WP/Table/BulkSubmitTableWidget.php | 12 ++++ inc/Smartling/WP/View/BulkSubmit.php | 23 +++++--- inc/Smartling/WP/View/ContentEditJob.php | 24 +++++--- js/app.js | 55 +++++++++++++++++-- 6 files changed, 94 insertions(+), 23 deletions(-) diff --git a/inc/Smartling/WP/Controller/BulkSubmitController.php b/inc/Smartling/WP/Controller/BulkSubmitController.php index b9cbbae69..c184d66f5 100644 --- a/inc/Smartling/WP/Controller/BulkSubmitController.php +++ b/inc/Smartling/WP/Controller/BulkSubmitController.php @@ -96,6 +96,7 @@ public function renderPage() $profile, $this->wpProxy, $this->nonceVerifier, + $applicableProfiles, ); $this->view($table); } diff --git a/inc/Smartling/WP/Controller/ContentEditJobController.php b/inc/Smartling/WP/Controller/ContentEditJobController.php index fc99e697d..f700862ce 100644 --- a/inc/Smartling/WP/Controller/ContentEditJobController.php +++ b/inc/Smartling/WP/Controller/ContentEditJobController.php @@ -302,6 +302,7 @@ public function box($attr) $this->view( [ 'profile' => $profile, + 'profiles' => $applicableProfiles, 'contentType' => $contentType, ] ); @@ -318,6 +319,7 @@ public function box($attr) $this->view( [ 'profile' => $profile, + 'profiles' => $applicableProfiles, 'contentType' => $contentType, ] ); diff --git a/inc/Smartling/WP/Table/BulkSubmitTableWidget.php b/inc/Smartling/WP/Table/BulkSubmitTableWidget.php index a65427b5c..82bed3e2f 100644 --- a/inc/Smartling/WP/Table/BulkSubmitTableWidget.php +++ b/inc/Smartling/WP/Table/BulkSubmitTableWidget.php @@ -71,6 +71,14 @@ public function getProfile(): ConfigurationProfileEntity return $this->profile; } + /** + * @return ConfigurationProfileEntity[] + */ + public function getApplicableProfiles(): array + { + return $this->applicableProfiles; + } + public function __construct( private AcfDynamicSupport $acfDynamicSupport, private ApiWrapperInterface $apiWrapper, @@ -82,7 +90,11 @@ public function __construct( protected ConfigurationProfileEntity $profile, protected WordpressFunctionProxyHelper $wpProxy, protected NonceVerifier $nonceVerifier, + protected array $applicableProfiles = [], ) { + if ([] === $this->applicableProfiles) { + $this->applicableProfiles = [$profile]; + } $this->setSource($_REQUEST); $filteredAllowedTypes = $this->getFilteredAllowedTypes(); diff --git a/inc/Smartling/WP/View/BulkSubmit.php b/inc/Smartling/WP/View/BulkSubmit.php index 659638864..bc66be591 100644 --- a/inc/Smartling/WP/View/BulkSubmit.php +++ b/inc/Smartling/WP/View/BulkSubmit.php @@ -52,22 +52,27 @@ display() ?>
getProfile()->getTargetLocales(); - ArrayHelper::sortLocales($locales); - $localesData = array_map(function($locale) { + $profilesData = array_map(function(\Smartling\Settings\ConfigurationProfileEntity $p) { + $pLocales = $p->getTargetLocales(); + ArrayHelper::sortLocales($pLocales); return [ - 'blogId' => $locale->getBlogId(), - 'label' => $locale->getLabel(), - 'smartlingLocale' => $locale->getSmartlingLocale(), - 'enabled' => $locale->isEnabled() + 'id' => $p->getId(), + 'name' => $p->getProfileName(), + 'locales' => array_values(array_map(fn($l) => [ + 'blogId' => $l->getBlogId(), + 'label' => $l->getLabel(), + 'smartlingLocale' => $l->getSmartlingLocale(), + 'enabled' => $l->isEnabled() + ], array_filter($pLocales, fn($l) => $l->isEnabled()))), ]; - }, array_filter($locales, fn($l) => $l->isEnabled())); + }, $data->getApplicableProfiles()); ?>
' + data-blog-id="siteHelper->getCurrentBlogId() ?>" + data-profiles='' data-ajax-url="" data-admin-url="" data-nonce="">
diff --git a/inc/Smartling/WP/View/ContentEditJob.php b/inc/Smartling/WP/View/ContentEditJob.php index 7906f5dac..1bc5fea9d 100644 --- a/inc/Smartling/WP/View/ContentEditJob.php +++ b/inc/Smartling/WP/View/ContentEditJob.php @@ -35,16 +35,21 @@ } } -$locales = $profile->getTargetLocales(); -ArrayHelper::sortLocales($locales); -$localesData = array_map(function($locale) { +$profiles = $data['profiles'] ?? [$profile]; +$profilesData = array_map(function(ConfigurationProfileEntity $p) { + $pLocales = $p->getTargetLocales(); + ArrayHelper::sortLocales($pLocales); return [ - 'blogId' => $locale->getBlogId(), - 'label' => $locale->getLabel(), - 'smartlingLocale' => $locale->getSmartlingLocale(), - 'enabled' => $locale->isEnabled() + 'id' => $p->getId(), + 'name' => $p->getProfileName(), + 'locales' => array_values(array_map(fn($l) => [ + 'blogId' => $l->getBlogId(), + 'label' => $l->getLabel(), + 'smartlingLocale' => $l->getSmartlingLocale(), + 'enabled' => $l->isEnabled() + ], array_filter($pLocales, fn($l) => $l->isEnabled()))), ]; -}, array_filter($locales, fn($l) => $l->isEnabled())); +}, $profiles); if (!$isBulkSubmitPage) : ?> @@ -56,7 +61,8 @@ data-bulk-submit="false" data-content-type="" data-content-id="" - data-locales='' + data-blog-id="siteHelper->getCurrentBlogId() ?>" + data-profiles='' data-ajax-url="" data-admin-url="" data-nonce=""> diff --git a/js/app.js b/js/app.js index c287321cf..954493649 100644 --- a/js/app.js +++ b/js/app.js @@ -1,8 +1,31 @@ const { render, createElement: el, useState, useEffect, useCallback } = wp.element; const { Button, Card, CardBody, CardHeader, TabPanel, TextControl, TextareaControl, CheckboxControl, SelectControl, Spinner, Notice, Flex, __experimentalVStack: VStack } = wp.components; -function JobWizard({ isBulkSubmitPage, contentType, contentId, locales, ajaxUrl, adminUrl, nonce }) { +function getStoredProfileId(blogId) { + try { + const stored = window.localStorage.getItem(`smartling_last_profile_${blogId}`); + return stored ? parseInt(stored, 10) : null; + } catch (e) { + return null; + } +} + +function setStoredProfileId(blogId, profileId) { + try { + window.localStorage.setItem(`smartling_last_profile_${blogId}`, String(profileId)); + } catch (e) { + // Ignore storage errors (private browsing, quota, etc.) - profile selection + // just won't be remembered across page loads. + } +} + +function JobWizard({ isBulkSubmitPage, contentType, contentId, profiles, blogId, ajaxUrl, adminUrl, nonce }) { const [activeTab, setActiveTab] = useState('new'); + const [selectedProfileId, setSelectedProfileId] = useState(() => { + const stored = getStoredProfileId(blogId); + return (stored !== null && profiles.some(p => p.id === stored)) ? stored : profiles[0]?.id; + }); + const locales = profiles.find(p => p.id === selectedProfileId)?.locales || []; const [jobs, setJobs] = useState([]); const [selectedJob, setSelectedJob] = useState(''); const [jobName, setJobName] = useState(''); @@ -33,7 +56,7 @@ function JobWizard({ isBulkSubmitPage, contentType, contentId, locales, ajaxUrl, action: 'smartling_job_api_proxy', _wpnonce: nonce, innerAction: 'list-jobs', - params: {} + params: { profileId: selectedProfileId } }); if (response.status === 200) { setJobs(response.data); @@ -43,12 +66,24 @@ function JobWizard({ isBulkSubmitPage, contentType, contentId, locales, ajaxUrl, } finally { setLoading(false); } - }, [adminUrl]); + }, [adminUrl, selectedProfileId]); useEffect(() => { loadJobs(); }, [loadJobs]); + const handleProfileChange = (val) => { + const newProfileId = parseInt(val, 10); + setSelectedProfileId(newProfileId); + setStoredProfileId(blogId, newProfileId); + const newLocales = (profiles.find(p => p.id === newProfileId)?.locales || []).map(l => l.blogId); + setSelectedLocales(prev => prev.filter(id => newLocales.includes(id))); + setSelectedJob(''); + setJobName(''); + setDescription(''); + setDueDate(''); + }; + const loadRelations = useCallback(async (type, id, level = 1) => { const localeList = locales.map(l => l.blogId).join(','); const url = `${ajaxUrl}?action=smartling-get-relations&id=${id}&content-type=${type}&targetBlogIds=${localeList}&_wpnonce=${encodeURIComponent(nonce)}`; @@ -250,6 +285,7 @@ function JobWizard({ isBulkSubmitPage, contentType, contentId, locales, ajaxUrl, const data = { _wpnonce: nonce, formAction: 'upload', + profileId: selectedProfileId, source: { contentType, id: isBulkSubmitPage ? [] : [contentId] }, job: { id: activeTab === 'new' ? '' : selectedJob, @@ -289,6 +325,7 @@ function JobWizard({ isBulkSubmitPage, contentType, contentId, locales, ajaxUrl, _wpnonce: nonce, innerAction: 'create-job', params: { + profileId: selectedProfileId, jobName, description, dueDate, @@ -387,6 +424,13 @@ function JobWizard({ isBulkSubmitPage, contentType, contentId, locales, ajaxUrl, ), el('div', {}, + tab.name !== 'instant' && profiles.length > 1 && el(SelectControl, { + label: 'Translation profile', + value: selectedProfileId, + options: profiles.map(p => ({ label: p.name, value: p.id })), + onChange: handleProfileChange + }), + el('fieldset', { style: { marginTop: '16px', border: '1px solid #ddd', padding: '12px', borderRadius: '4px' } }, el('legend', { style: { fontWeight: 600, padding: '0 8px' } }, 'Target Locales'), el('div', { style: { display: 'flex', gap: '8px', marginBottom: '8px' } }, @@ -508,13 +552,14 @@ if (document.getElementById('smartling-app')) { const isBulkSubmitPage = container.dataset.bulkSubmit === 'true'; const contentType = container.dataset.contentType || ''; const contentId = parseInt(container.dataset.contentId) || 0; - const locales = JSON.parse(container.dataset.locales || '[]'); + const profiles = JSON.parse(container.dataset.profiles || '[]'); + const blogId = parseInt(container.dataset.blogId) || 0; const ajaxUrl = container.dataset.ajaxUrl || ''; const adminUrl = container.dataset.adminUrl || ''; const nonce = container.dataset.nonce || ''; render( - el(JobWizard, { isBulkSubmitPage, contentType, contentId, locales, ajaxUrl, adminUrl, nonce }), + el(JobWizard, { isBulkSubmitPage, contentType, contentId, profiles, blogId, ajaxUrl, adminUrl, nonce }), container ); } From 71d7e48ff0198439ce6854bdf5d9030fb38be5ca Mon Sep 17 00:00:00 2001 From: Vitalii Solovei Date: Tue, 6 Oct 2026 12:37:02 +0200 Subject: [PATCH 05/14] auto-refresh the download widget after queuing an upload (WP-1022) The per-post "Smartling Widget" (side metabox) only showed a link to a target placeholder post once its row had been rendered server-side on page load, so after queuing a new upload from the job wizard the user had to manually reload the page to get a link to the freshly created placeholder. Add an AJAX endpoint that re-renders that metabox's markup on demand, and have the job wizard poll it with a short backoff after a successful (non bulk-submit) upload, swapping in the refreshed widget until every requested target locale has an edit link or the poll window runs out. Co-Authored-By: Claude Sonnet 5 --- .../PostBasedWidgetControllerStd.php | 32 ++++++++++++++ js/app.js | 44 +++++++++++++++++++ 2 files changed, 76 insertions(+) diff --git a/inc/Smartling/WP/Controller/PostBasedWidgetControllerStd.php b/inc/Smartling/WP/Controller/PostBasedWidgetControllerStd.php index 6a6731f36..acf175b22 100644 --- a/inc/Smartling/WP/Controller/PostBasedWidgetControllerStd.php +++ b/inc/Smartling/WP/Controller/PostBasedWidgetControllerStd.php @@ -407,9 +407,41 @@ public function register(): void add_action('save_post', [$this, 'save']); // old logic 2 be refactored add_action('wp_ajax_' . 'smartling_force_download_handler', [$this, 'ajaxDownloadHandler']); add_action('wp_ajax_' . 'smartling_upload_handler', [$this, 'ajaxUploadHandler']); + add_action('wp_ajax_' . 'smartling_refresh_post_widget', [$this, 'ajaxRefreshWidgetHandler']); } } + /** + * Re-renders the widget for a post so the caller can swap it into the DOM, picking up + * submissions/target placeholders created after the page was first loaded (e.g. by the + * upload job wizard) without a full page reload. + */ + public function ajaxRefreshWidgetHandler(): void + { + if (check_ajax_referer(self::AJAX_NONCE_ACTION, '_wpnonce', false) === false) { + $this->getLogger()->warning(sprintf('Invalid nonce for action "%s" from userId=%d', self::AJAX_NONCE_ACTION, get_current_user_id())); + wp_send_json(['status' => self::RESPONSE_AJAX_STATUS_FAIL, 'message' => 'Invalid nonce'], 403); + return; + } + if (!current_user_can(SmartlingUserCapabilities::SMARTLING_CAPABILITY_WIDGET_CAP)) { + $this->getLogger()->warning(sprintf('User %d lacks capability "%s"', get_current_user_id(), SmartlingUserCapabilities::SMARTLING_CAPABILITY_WIDGET_CAP)); + wp_send_json(['status' => self::RESPONSE_AJAX_STATUS_FAIL, 'message' => 'Insufficient permissions'], 403); + return; + } + + $post = get_post((int)($_POST['postId'] ?? 0)); + if (!($post instanceof \WP_Post)) { + wp_send_json(['status' => self::RESPONSE_AJAX_STATUS_FAIL, 'message' => 'Post not found'], 404); + return; + } + + ob_start(); + $this->preView($post); + $html = ob_get_clean(); + + wp_send_json(['status' => self::RESPONSE_AJAX_STATUS_SUCCESS, 'html' => $html]); + } + /** * @var SmartlingCore */ diff --git a/js/app.js b/js/app.js index 954493649..3869a7b77 100644 --- a/js/app.js +++ b/js/app.js @@ -19,6 +19,47 @@ function setStoredProfileId(blogId, profileId) { } } +async function refreshDownloadWidgetUntilReady(adminUrl, postId, targetBlogIds) { + if (!document.getElementById('smartling-post-widget') || !targetBlogIds.length) { + return; + } + + const DELAYS = [3000, 5000, 10000, 15000, 15000]; + for (const delay of DELAYS) { + await new Promise(resolve => setTimeout(resolve, delay)); + + let response; + try { + response = await jQuery.post(adminUrl, { + action: 'smartling_refresh_post_widget', + postId, + _wpnonce: (typeof smartlingConnector !== 'undefined' ? smartlingConnector.nonce : '') + }); + } catch (e) { + continue; + } + if (response.status !== 'SUCCESS' || !response.html) { + continue; + } + + const widget = document.getElementById('smartling-post-widget'); + if (!widget) { + return; + } + widget.outerHTML = response.html; + + const updated = document.getElementById('smartling-post-widget'); + const allReady = targetBlogIds.every(blogId => { + const blogInput = updated?.querySelector(`input[name="smartling[locales][${blogId}][blog]"]`); + const row = blogInput?.closest('.smtPostWidget-rowWrapper'); + return row?.querySelector('.smtPostWidget-row a') != null; + }); + if (allReady) { + return; + } + } +} + function JobWizard({ isBulkSubmitPage, contentType, contentId, profiles, blogId, ajaxUrl, adminUrl, nonce }) { const [activeTab, setActiveTab] = useState('new'); const [selectedProfileId, setSelectedProfileId] = useState(() => { @@ -346,6 +387,9 @@ function JobWizard({ isBulkSubmitPage, contentType, contentId, profiles, blogId, throw new Error(submissionResponse.message?.global || 'Failed to add content to upload queue.'); } setSuccess('Content successfully added to upload queue.'); + if (!isBulkSubmitPage) { + refreshDownloadWidgetUntilReady(adminUrl, contentId, selectedLocales); + } } catch (e) { setError(e.message || 'Failed adding content to upload queue.'); } finally { From 59ecdc863cfb10fbba207f7ccd7a005a3d3d1a7e Mon Sep 17 00:00:00 2001 From: Vitalii Solovei Date: Wed, 7 Oct 2026 13:21:18 +0200 Subject: [PATCH 06/14] resolve requested profile centrally and harden profile selection (WP-1022) Co-Authored-By: Claude Sonnet 5.5 --- .../Models/UserTranslationRequest.php | 25 +++- .../ContentRelationsDiscoveryService.php | 64 +++----- .../Settings/ConfigurationProfileEntity.php | 21 +++ inc/Smartling/Settings/SettingsManager.php | 38 +++++ .../Controller/ContentEditJobController.php | 40 ++--- .../InstantTranslationController.php | 28 +++- .../PostBasedWidgetControllerStd.php | 6 + .../WP/Table/BulkSubmitTableWidget.php | 5 +- inc/Smartling/WP/View/BulkSubmit.php | 19 +-- inc/Smartling/WP/View/ContentEditJob.php | 17 +-- .../WP/View/post-based-content-type.php | 8 +- inc/config/services.yml | 1 + js/app.js | 17 ++- tests/Models/TranslationRequestTest.php | 37 ++++- .../ContentRelationsDiscoveryServiceTest.php | 51 ++----- .../Settings/SettingsManagerTest.php | 82 ++++++++++ .../ContentEditJobControllerTest.php | 141 ------------------ .../InstantTranslationControllerTest.php | 30 +++- 18 files changed, 321 insertions(+), 309 deletions(-) delete mode 100644 tests/Smartling/WP/Controller/ContentEditJobControllerTest.php diff --git a/inc/Smartling/Models/UserTranslationRequest.php b/inc/Smartling/Models/UserTranslationRequest.php index 2c644b502..a6afeca36 100644 --- a/inc/Smartling/Models/UserTranslationRequest.php +++ b/inc/Smartling/Models/UserTranslationRequest.php @@ -13,10 +13,10 @@ class UserTranslationRequest private array $relations; private array $targetBlogIds; private JobInformation $jobInformation; - private int $profileId; + private ?int $profileId; private array $ids; - public function __construct(int $contentId, string $contentType, array $relations, array $targetBlogIds, JobInformation $jobInformation, int $profileId, array $ids = [], string $description = '') + public function __construct(int $contentId, string $contentType, array $relations, array $targetBlogIds, JobInformation $jobInformation, ?int $profileId = null, array $ids = [], string $description = '') { $this->contentId = $contentId; $this->contentType = $contentType; @@ -62,7 +62,7 @@ public function getJobInformation(): JobInformation return $this->jobInformation; } - public function getProfileId(): int + public function getProfileId(): ?int { return $this->profileId; } @@ -84,7 +84,7 @@ public static function fromArray(array $array): self $array['relations'] ?? [], explode(',', $array['targetBlogIds']), new JobInformation($array['job']['id'], $array['job']['authorize'] === 'true', $array['job']['name'], $array['job']['description'], $array['job']['dueDate'], $array['job']['timeZone']), - (int)$array['profileId'], + self::parseProfileId($array['profileId'] ?? null), $ids, $array['description'] ?? (count($ids) > 0 ? 'From Bulk Submit' : 'From Widget'), ); @@ -130,9 +130,22 @@ private static function validate(array $array): void if (!array_key_exists('timeZone', $array['job'])) { throw new \InvalidArgumentException('Job time zone required'); } - if (!array_key_exists('profileId', $array)) { - throw new \InvalidArgumentException('Profile id required'); + } + + public static function parseProfileId(mixed $value): ?int + { + if ($value === null || $value === '') { + return null; } + if (!is_int($value) && !(is_string($value) && ctype_digit($value))) { + throw new \InvalidArgumentException('Profile id must be a positive integer'); + } + $profileId = (int)$value; + if ($profileId < 1) { + throw new \InvalidArgumentException('Profile id must be a positive integer'); + } + + return $profileId; } private static function toIntegerArray(array $ids): array diff --git a/inc/Smartling/Services/ContentRelationsDiscoveryService.php b/inc/Smartling/Services/ContentRelationsDiscoveryService.php index bc3cafe48..c02512a21 100644 --- a/inc/Smartling/Services/ContentRelationsDiscoveryService.php +++ b/inc/Smartling/Services/ContentRelationsDiscoveryService.php @@ -13,7 +13,6 @@ use Smartling\DbAl\UploadQueueManager; use Smartling\DbAl\WordpressContentEntities\EntityWithMetadata; use Smartling\Exception\EntityNotFoundException; -use Smartling\Exception\SmartlingDbException; use Smartling\Exception\SmartlingGutenbergParserNotFoundException; use Smartling\Exception\SmartlingHumanReadableException; use Smartling\Extensions\Acf\AcfDynamicSupport; @@ -89,6 +88,7 @@ public function bulkUpload( ConfigurationProfileEntity $profile, array $targetBlogIds, bool $enqueue = true, + bool $stampProfile = true, ): array { $this->getLogger()->debug("Bulk upload request, contentIds=" . implode(',', $contentIds)); $queueIds = []; @@ -96,7 +96,8 @@ public function bulkUpload( foreach ($targetBlogIds as $targetBlogId) { foreach ($contentIds as $id) { $submission = $this->submissionManager->findTargetBlogSubmission($contentType, $currentBlogId, $id, $targetBlogId); - if ($submission === null) { + $isNew = $submission === null; + if ($isNew) { $submission = $this->submissionManager->getSubmissionEntity($contentType, $currentBlogId, $id, $targetBlogId, $this->localizationPluginProxy); $title = $this->getTitle($submission); if ($title !== '') { @@ -107,11 +108,13 @@ 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()); + // Bulk-submitting is an explicit new translation request: (re)stamp with the profile it was requested with. + // Without a resolved profile, existing submissions keep theirs and new ones stay unstamped. + if ($stampProfile) { + $submission->setConfigurationProfileId($profile->getId()); + } elseif ($isNew) { + $submission->setConfigurationProfileId(null); + } $submission = $this->submissionManager->storeEntity($submission); $queueIds[] = $submission->getId(); $this->logSubmissionCreated($submission, 'Bulk upload request', $jobInfo); @@ -134,6 +137,7 @@ public function bulkUpload( $profile, $targetBlogIds, false, + $stampProfile, )); } if ($enqueue) { @@ -151,34 +155,12 @@ public function bulkUpload( return $queueIds; } - /** - * Resolves the profile the user explicitly chose for this request, rather than whichever - * one happens to be flagged active. Falls back to today's active-profile behavior if the - * requested id doesn't exist, belongs to a different blog, or isn't active - this can - * happen with stale client-side data (profile deactivated/deleted after the page loaded) - * and shouldn't hard-fail the whole request. - * - * @throws SmartlingDbException - */ - private function resolveRequestedProfile(int $requestedProfileId, int $curBlogId): ConfigurationProfileEntity - { - $profile = ArrayHelper::first($this->settingsManager->getEntityById($requestedProfileId)); - if ($profile instanceof ConfigurationProfileEntity) { - if ($profile->getSourceLocale()->getBlogId() === $curBlogId && 1 === $profile->getIsActive()) { - return $profile; - } - $this->getLogger()->warning("Requested profileId=$requestedProfileId is not an active profile for blogId=$curBlogId, falling back to active profile"); - } else { - $this->getLogger()->warning("Requested profileId=$requestedProfileId not found, falling back to active profile"); - } - - return $this->settingsManager->getSingleSettingsProfile($curBlogId); - } - public function createSubmissions(UserTranslationRequest $request): void { $curBlogId = $this->wordpressProxy->get_current_blog_id(); - $profile = $this->resolveRequestedProfile($request->getProfileId(), $curBlogId); + $requestedProfile = $this->settingsManager->resolveRequestedProfile($request->getProfileId(), $curBlogId); + $stampProfile = $requestedProfile !== null; + $profile = $requestedProfile ?? $this->settingsManager->getSingleSettingsProfile($curBlogId); $job = $request->getJobInformation(); $jobInfo = new JobEntity($job->getName(), $job->getId(), $profile->getProjectId()); @@ -190,6 +172,8 @@ public function createSubmissions(UserTranslationRequest $request): void $jobInfo, $profile, $request->getTargetBlogIds(), + true, + $stampProfile, ); return; } @@ -255,11 +239,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()); + // Resubmitting an existing submission is an explicit new translation request, so restamp it with the + // profile it was requested with. Without a resolved profile it keeps whatever it had. + if ($stampProfile) { + $submission->setConfigurationProfileId($profile->getId()); + } $submission = $this->storeWithJobInfo($submission, $jobInfo, $request->getDescription()); $fileUris[] = $submission->getFileUri(); $queueIds[] = $submission->getId(); @@ -267,10 +251,8 @@ 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(); + // New submissions built from this template bypass getSubmissionEntity(), stamp them with the requested profile. + $submissionTemplateArray[SubmissionEntity::FIELD_CONFIGURATION_PROFILE_ID] = $stampProfile ? $profile->getId() : null; foreach ($sources as $source) { $submissionArray = array_merge($submissionTemplateArray, [ diff --git a/inc/Smartling/Settings/ConfigurationProfileEntity.php b/inc/Smartling/Settings/ConfigurationProfileEntity.php index 4de78c6c9..96b245d20 100644 --- a/inc/Smartling/Settings/ConfigurationProfileEntity.php +++ b/inc/Smartling/Settings/ConfigurationProfileEntity.php @@ -3,6 +3,7 @@ namespace Smartling\Settings; use Smartling\Base\SmartlingEntityAbstract; +use Smartling\Helpers\ArrayHelper; use Smartling\Vendor\Psr\Log\LoggerInterface; use Smartling\WP\Controller\ConfigurationProfileFormController as Form; @@ -443,4 +444,24 @@ public function toArraySafe(): array unset($struct['secret_key']); return $struct; } + + /** + * Data for the job wizard profile select and its target locale list. + */ + public function toWizardArray(): array + { + $locales = $this->getTargetLocales(); + ArrayHelper::sortLocales($locales); + + return [ + 'id' => $this->getId(), + 'name' => $this->getProfileName(), + 'locales' => array_values(array_map(static fn(TargetLocale $l) => [ + 'blogId' => $l->getBlogId(), + 'label' => $l->getLabel(), + 'smartlingLocale' => $l->getSmartlingLocale(), + 'enabled' => $l->isEnabled(), + ], array_filter($locales, static fn(TargetLocale $l) => $l->isEnabled()))), + ]; + } } diff --git a/inc/Smartling/Settings/SettingsManager.php b/inc/Smartling/Settings/SettingsManager.php index 5d79a55ba..40b17e22e 100644 --- a/inc/Smartling/Settings/SettingsManager.php +++ b/inc/Smartling/Settings/SettingsManager.php @@ -119,6 +119,44 @@ public function getSingleSettingsProfile(int $mainBlogId): ConfigurationProfileE throw new SmartlingDbException($message); } + /** + * Resolves the profile for a translation request. + * An explicitly requested profile must exist, belong to the blog and be active, otherwise the request is rejected. + * Without an explicit profile, the blog's only active profile is used; with none or several, null is returned. + * + * @throws SmartlingDbException + */ + public function resolveRequestedProfile(?int $requestedProfileId, int $blogId): ?ConfigurationProfileEntity + { + if ($requestedProfileId !== null) { + $profile = ArrayHelper::first($this->getEntityById($requestedProfileId)); + if (!$profile instanceof ConfigurationProfileEntity) { + $message = "Requested profileId=$requestedProfileId not found"; + $this->getLogger()->warning($message); + throw new SmartlingDbException($message); + } + if ($profile->getSourceLocale()->getBlogId() !== $blogId || 1 !== $profile->getIsActive()) { + $message = "Requested profileId=$requestedProfileId is not an active profile for blogId=$blogId"; + $this->getLogger()->warning($message); + throw new SmartlingDbException($message); + } + $this->getLogger()->debug("Using requested profileId=$requestedProfileId for blogId=$blogId"); + + return $profile; + } + + $profiles = $this->findEntityByMainLocale($blogId); + if (1 === count($profiles)) { + $profile = ArrayHelper::first($profiles); + $this->getLogger()->debug("No profile requested, using the only active profileId={$profile->getId()} for blogId=$blogId"); + + return $profile; + } + $this->getLogger()->debug('No profile requested and ' . count($profiles) . " active profiles found for blogId=$blogId, profile will not be stored"); + + return null; + } + /** * 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. diff --git a/inc/Smartling/WP/Controller/ContentEditJobController.php b/inc/Smartling/WP/Controller/ContentEditJobController.php index f700862ce..af92547e2 100644 --- a/inc/Smartling/WP/Controller/ContentEditJobController.php +++ b/inc/Smartling/WP/Controller/ContentEditJobController.php @@ -18,7 +18,7 @@ use Smartling\Helpers\SiteHelper; use Smartling\Helpers\SmartlingUserCapabilities; use Smartling\Helpers\WordpressFunctionProxyHelper; -use Smartling\Settings\ConfigurationProfileEntity; +use Smartling\Models\UserTranslationRequest; use Smartling\Settings\SettingsManager; use Smartling\Submissions\SubmissionManager; use Smartling\Vendor\Smartling\Jobs\JobStatus; @@ -83,32 +83,6 @@ public function setServedContentType($servedContentType) $this->servedContentType = $servedContentType; } - /** - * Resolves the profile the user explicitly chose (e.g. from a profile dropdown), rather - * than whichever one happens to be flagged active. Falls back to today's active-profile - * behavior if no profile was requested, or the requested id doesn't exist, belongs to a - * different blog, or isn't active - this can happen with stale client-side data (profile - * deactivated/deleted after the page loaded) and shouldn't hard-fail the request. - * - * @throws SmartlingDbException - */ - private function resolveRequestedProfile(?int $requestedProfileId, int $curBlogId): ConfigurationProfileEntity - { - if ($requestedProfileId !== null) { - $profile = ArrayHelper::first($this->settingsManager->getEntityById($requestedProfileId)); - if ($profile instanceof ConfigurationProfileEntity) { - if ($profile->getSourceLocale()->getBlogId() === $curBlogId && 1 === $profile->getIsActive()) { - return $profile; - } - $this->getLogger()->warning("Requested profileId=$requestedProfileId is not an active profile for blogId=$curBlogId, falling back to active profile"); - } else { - $this->getLogger()->warning("Requested profileId=$requestedProfileId not found, falling back to active profile"); - } - } - - return $this->settingsManager->getSingleSettingsProfile($curBlogId); - } - public function initJobApiProxy(): void { add_action('wp_ajax_' . self::SMARTLING_JOB_API_PROXY, function () { @@ -130,8 +104,16 @@ public function initJobApiProxy(): void ]; $params = &$data['params']; - $requestedProfileId = isset($params['profileId']) ? (int)$params['profileId'] : null; - $profile = $this->resolveRequestedProfile($requestedProfileId, $this->siteHelper->getCurrentBlogId()); + try { + $blogId = $this->siteHelper->getCurrentBlogId(); + $profile = $this->settingsManager->resolveRequestedProfile( + UserTranslationRequest::parseProfileId($params['profileId'] ?? null), + $blogId, + ) ?? $this->settingsManager->getSingleSettingsProfile($blogId); + } catch (\InvalidArgumentException | SmartlingDbException $e) { + $this->wpProxy->wp_send_json(['status' => 400, 'message' => ['profileId' => $e->getMessage()]], 400); + return; + } $validateRequires = static function ($fieldName) use (&$result, $params) { $value = trim($params[$fieldName] ?? ''); diff --git a/inc/Smartling/WP/Controller/InstantTranslationController.php b/inc/Smartling/WP/Controller/InstantTranslationController.php index 0c283eea5..6066dc9ea 100644 --- a/inc/Smartling/WP/Controller/InstantTranslationController.php +++ b/inc/Smartling/WP/Controller/InstantTranslationController.php @@ -2,6 +2,7 @@ namespace Smartling\WP\Controller; +use Smartling\Exception\SmartlingDbException; use Smartling\FTS\FtsService; use Smartling\Helpers\AjaxSecurityChecker; use Smartling\Helpers\DateTimeHelper; @@ -9,6 +10,8 @@ use Smartling\Helpers\LoggerSafeTrait; use Smartling\Helpers\SmartlingUserCapabilities; use Smartling\Helpers\WordpressFunctionProxyHelper; +use Smartling\Models\UserTranslationRequest; +use Smartling\Settings\SettingsManager; use Smartling\Submissions\SubmissionEntity; use Smartling\Submissions\SubmissionFactory; use Smartling\Submissions\SubmissionManager; @@ -28,6 +31,7 @@ public function __construct( private FileUriHelper $fileUriHelper, private WordpressFunctionProxyHelper $wpProxy, private AjaxSecurityChecker $ajaxSecurity, + private SettingsManager $settingsManager, ) { } @@ -68,12 +72,23 @@ public function handleRequestTranslation(): void $sourceBlogId = $this->wpProxy->get_current_blog_id(); + try { + $profileId = $this->settingsManager->resolveRequestedProfile( + UserTranslationRequest::parseProfileId($_POST['profileId'] ?? null), + $sourceBlogId, + )?->getId(); + } catch (\InvalidArgumentException | SmartlingDbException $e) { + $this->wpProxy->wp_send_json_error(['message' => $e->getMessage()], 400); + return; + } + $allSubmissions = $this->buildSubmissions( $contentType, $contentId, $sourceBlogId, $targetBlogIds, $relations, + $profileId, ); if (empty($allSubmissions)) { @@ -208,7 +223,8 @@ private function buildSubmissions( int $contentId, int $sourceBlogId, array $targetBlogIds, - array $relations + array $relations, + ?int $profileId, ): array { $submissions = []; @@ -218,6 +234,7 @@ private function buildSubmissions( $targetBlogId, $contentType, $contentId, + $profileId, ); if ($mainSubmission !== null) { $submissions[] = $mainSubmission; @@ -229,7 +246,8 @@ private function buildSubmissions( $sourceBlogId, $targetBlogId, $source['type'], - $source['id'] + $source['id'], + $profileId, ); if ($relatedSubmission !== null) { $submissions[] = $relatedSubmission; @@ -275,7 +293,8 @@ private function getOrCreateSubmission( int $sourceBlogId, int $targetBlogId, string $contentType, - int $contentId + int $contentId, + ?int $profileId, ): ?SubmissionEntity { try { $submission = $this->submissionManager->findOne([ @@ -300,6 +319,9 @@ private function getOrCreateSubmission( $submission->setStatus(SubmissionEntity::SUBMISSION_STATUS_NEW); } + if ($profileId !== null) { + $submission->setConfigurationProfileId($profileId); + } $submission->setStatus(SubmissionEntity::SUBMISSION_STATUS_IN_PROGRESS); return $this->submissionManager->storeEntity($submission); } catch (\Exception $e) { diff --git a/inc/Smartling/WP/Controller/PostBasedWidgetControllerStd.php b/inc/Smartling/WP/Controller/PostBasedWidgetControllerStd.php index acf175b22..f90f24f8c 100644 --- a/inc/Smartling/WP/Controller/PostBasedWidgetControllerStd.php +++ b/inc/Smartling/WP/Controller/PostBasedWidgetControllerStd.php @@ -434,6 +434,11 @@ public function ajaxRefreshWidgetHandler(): void wp_send_json(['status' => self::RESPONSE_AJAX_STATUS_FAIL, 'message' => 'Post not found'], 404); return; } + if (!current_user_can('edit_post', $post->ID)) { + $this->getLogger()->warning(sprintf('User %d cannot edit post %d', get_current_user_id(), $post->ID)); + wp_send_json(['status' => self::RESPONSE_AJAX_STATUS_FAIL, 'message' => 'Insufficient permissions'], 403); + return; + } ob_start(); $this->preView($post); @@ -504,6 +509,7 @@ public function preView($post) 'submissions' => $submissions, 'post' => $post, 'profile' => ArrayHelper::first($profile), + 'profiles' => $profile, ] ); } else { diff --git a/inc/Smartling/WP/Table/BulkSubmitTableWidget.php b/inc/Smartling/WP/Table/BulkSubmitTableWidget.php index 82bed3e2f..aff4f3925 100644 --- a/inc/Smartling/WP/Table/BulkSubmitTableWidget.php +++ b/inc/Smartling/WP/Table/BulkSubmitTableWidget.php @@ -90,11 +90,8 @@ public function __construct( protected ConfigurationProfileEntity $profile, protected WordpressFunctionProxyHelper $wpProxy, protected NonceVerifier $nonceVerifier, - protected array $applicableProfiles = [], + protected array $applicableProfiles, ) { - if ([] === $this->applicableProfiles) { - $this->applicableProfiles = [$profile]; - } $this->setSource($_REQUEST); $filteredAllowedTypes = $this->getFilteredAllowedTypes(); diff --git a/inc/Smartling/WP/View/BulkSubmit.php b/inc/Smartling/WP/View/BulkSubmit.php index bc66be591..c6290c4a1 100644 --- a/inc/Smartling/WP/View/BulkSubmit.php +++ b/inc/Smartling/WP/View/BulkSubmit.php @@ -52,27 +52,14 @@ display() ?>
getTargetLocales(); - ArrayHelper::sortLocales($pLocales); - return [ - 'id' => $p->getId(), - 'name' => $p->getProfileName(), - 'locales' => array_values(array_map(fn($l) => [ - 'blogId' => $l->getBlogId(), - 'label' => $l->getLabel(), - 'smartlingLocale' => $l->getSmartlingLocale(), - 'enabled' => $l->isEnabled() - ], array_filter($pLocales, fn($l) => $l->isEnabled()))), - ]; - }, $data->getApplicableProfiles()); + $profilesData = array_map(static fn(\Smartling\Settings\ConfigurationProfileEntity $p) => $p->toWizardArray(), $data->getApplicableProfiles()); ?>
' + data-blog-id="siteHelper->getCurrentBlogId() ?>" + data-profiles='' data-ajax-url="" data-admin-url="" data-nonce="">
diff --git a/inc/Smartling/WP/View/ContentEditJob.php b/inc/Smartling/WP/View/ContentEditJob.php index 1bc5fea9d..9127d2661 100644 --- a/inc/Smartling/WP/View/ContentEditJob.php +++ b/inc/Smartling/WP/View/ContentEditJob.php @@ -36,20 +36,7 @@ } $profiles = $data['profiles'] ?? [$profile]; -$profilesData = array_map(function(ConfigurationProfileEntity $p) { - $pLocales = $p->getTargetLocales(); - ArrayHelper::sortLocales($pLocales); - return [ - 'id' => $p->getId(), - 'name' => $p->getProfileName(), - 'locales' => array_values(array_map(fn($l) => [ - 'blogId' => $l->getBlogId(), - 'label' => $l->getLabel(), - 'smartlingLocale' => $l->getSmartlingLocale(), - 'enabled' => $l->isEnabled() - ], array_filter($pLocales, fn($l) => $l->isEnabled()))), - ]; -}, $profiles); +$profilesData = array_map(static fn(ConfigurationProfileEntity $p) => $p->toWizardArray(), $profiles); if (!$isBulkSubmitPage) : ?> @@ -61,7 +48,7 @@ data-bulk-submit="false" data-content-type="" data-content-id="" - data-blog-id="siteHelper->getCurrentBlogId() ?>" + data-blog-id="siteHelper->getCurrentBlogId() ?>" data-profiles='' data-ajax-url="" data-admin-url="" diff --git a/inc/Smartling/WP/View/post-based-content-type.php b/inc/Smartling/WP/View/post-based-content-type.php index d426a8dde..bb4085d5a 100644 --- a/inc/Smartling/WP/View/post-based-content-type.php +++ b/inc/Smartling/WP/View/post-based-content-type.php @@ -18,7 +18,13 @@ /** * @var TargetLocale[] $locales */ -$locales = $data['profile']->getTargetLocales(); +$locales = []; +foreach ($data['profiles'] ?? [$data['profile']] as $profile) { + foreach ($profile->getTargetLocales() as $locale) { + $locales[$locale->getBlogId()] ??= $locale; + } +} +$locales = array_values($locales); $filteredLocales = []; diff --git a/inc/config/services.yml b/inc/config/services.yml index 9aa4ab025..f2d53f40b 100644 --- a/inc/config/services.yml +++ b/inc/config/services.yml @@ -516,6 +516,7 @@ services: - "@file.uri.helper" - "@wp.proxy" - "@helper.ajax-security" + - "@manager.settings" wp.upload-queue-count: class: Smartling\WP\Controller\UploadQueueCountController diff --git a/js/app.js b/js/app.js index 3869a7b77..f1b7e1ec9 100644 --- a/js/app.js +++ b/js/app.js @@ -4,7 +4,8 @@ const { Button, Card, CardBody, CardHeader, TabPanel, TextControl, TextareaContr function getStoredProfileId(blogId) { try { const stored = window.localStorage.getItem(`smartling_last_profile_${blogId}`); - return stored ? parseInt(stored, 10) : null; + const profileId = stored ? parseInt(stored, 10) : NaN; + return Number.isNaN(profileId) ? null : profileId; } catch (e) { return null; } @@ -213,6 +214,7 @@ function JobWizard({ isBulkSubmitPage, contentType, contentId, profiles, blogId, const response = await jQuery.post(ajaxUrl, { action: 'smartling_instant_translation', _wpnonce: nonce, + profileId: selectedProfileId, contentType: contentType, contentId: contentId, targetBlogIds: selectedLocales, @@ -468,7 +470,7 @@ function JobWizard({ isBulkSubmitPage, contentType, contentId, profiles, blogId, ), el('div', {}, - tab.name !== 'instant' && profiles.length > 1 && el(SelectControl, { + profiles.length > 1 && el(SelectControl, { label: 'Translation profile', value: selectedProfileId, options: profiles.map(p => ({ label: p.name, value: p.id })), @@ -602,9 +604,12 @@ if (document.getElementById('smartling-app')) { const adminUrl = container.dataset.adminUrl || ''; const nonce = container.dataset.nonce || ''; - render( - el(JobWizard, { isBulkSubmitPage, contentType, contentId, profiles, blogId, ajaxUrl, adminUrl, nonce }), - container - ); + // Nothing to offer without an active profile + if (profiles.length > 0) { + render( + el(JobWizard, { isBulkSubmitPage, contentType, contentId, profiles, blogId, ajaxUrl, adminUrl, nonce }), + container + ); + } } diff --git a/tests/Models/TranslationRequestTest.php b/tests/Models/TranslationRequestTest.php index f76c4cd83..4553b26f7 100644 --- a/tests/Models/TranslationRequestTest.php +++ b/tests/Models/TranslationRequestTest.php @@ -49,14 +49,47 @@ public function testFromArray() $this->assertEquals(9, $x->getProfileId()); } - public function testFromArrayRequiresProfileId() + public function testFromArrayProfileIdIsOptional() { - $this->expectException(\InvalidArgumentException::class); $array = $this->buildArray(); unset($array['profileId']); + self::assertNull(UserTranslationRequest::fromArray($array)->getProfileId()); + } + + public function testFromArrayRejectsInvalidProfileId() + { + $this->expectException(\InvalidArgumentException::class); + $array = $this->buildArray(); + $array['profileId'] = 'abc'; UserTranslationRequest::fromArray($array); } + public function testParseProfileIdAcceptsMissingValue() + { + self::assertNull(UserTranslationRequest::parseProfileId(null)); + self::assertNull(UserTranslationRequest::parseProfileId('')); + } + + public function testParseProfileIdAcceptsPositiveInteger() + { + self::assertSame(5, UserTranslationRequest::parseProfileId('5')); + self::assertSame(5, UserTranslationRequest::parseProfileId(5)); + } + + /** + * @dataProvider invalidProfileIdProvider + */ + public function testParseProfileIdRejectsInvalidValues(mixed $value) + { + $this->expectException(\InvalidArgumentException::class); + UserTranslationRequest::parseProfileId($value); + } + + public static function invalidProfileIdProvider(): array + { + return [['abc'], ['0'], [0], [-3], ['-3'], ['1.5'], [[5]]]; + } + public function testFromArrayBulkUploadWithEmptySourceId() { $targetBlogId = 2; diff --git a/tests/Services/ContentRelationsDiscoveryServiceTest.php b/tests/Services/ContentRelationsDiscoveryServiceTest.php index 838317f92..ff2db4b21 100644 --- a/tests/Services/ContentRelationsDiscoveryServiceTest.php +++ b/tests/Services/ContentRelationsDiscoveryServiceTest.php @@ -178,7 +178,7 @@ public function testCreateSubmissionsHandlerStampsConfigurationProfileOnExisting $apiWrapper = $this->createMock(ApiWrapper::class); $settingsManager = $this->createMock(SettingsManager::class); - $settingsManager->method('getSingleSettingsProfile')->willReturn($profile); + $settingsManager->method('resolveRequestedProfile')->with(5, $sourceBlogId)->willReturn($profile); $siteHelper = $this->createMock(SiteHelper::class); $siteHelper->method('getCurrentBlogId')->willReturn($sourceBlogId); @@ -216,18 +216,10 @@ public function testCreateSubmissionsHandlerStampsConfigurationProfileOnExisting ])); } - /** - * @dataProvider invalidRequestedProfileProvider - */ - public function testCreateSubmissionsFallsBackToActiveProfileWhenRequestedOneIsNotUsable(?ConfigurationProfileEntity $requestedProfile) + public function testCreateSubmissionsDoesNotStampProfileWhenNoneCouldBeResolved() { $sourceBlogId = 1; $sourceId = 48; - $contentType = 'post'; - $targetBlogId = 2; - $jobName = 'Job Name'; - $jobUid = 'abcdef123456'; - $requestedProfileId = 5; $activeProfile = $this->createMock(ConfigurationProfileEntity::class); $activeProfile->method('getProjectId')->willReturn('activeProjectUid'); @@ -235,12 +227,12 @@ public function testCreateSubmissionsFallsBackToActiveProfileWhenRequestedOneIsN $apiWrapper = $this->createMock(ApiWrapper::class); $apiWrapper->expects($this->once())->method('createAuditLogRecord')->willReturnCallback( function (ConfigurationProfileEntity $configurationProfile) use ($activeProfile): void { - $this->assertSame($activeProfile, $configurationProfile, 'Must fall back to the active profile, not the unusable requested one'); + $this->assertSame($activeProfile, $configurationProfile); }, ); $settingsManager = $this->createMock(SettingsManager::class); - $settingsManager->method('getEntityById')->with($requestedProfileId)->willReturn($requestedProfile === null ? [] : [$requestedProfile]); + $settingsManager->method('resolveRequestedProfile')->with(null, $sourceBlogId)->willReturn(null); $settingsManager->expects(self::once())->method('getSingleSettingsProfile')->with($sourceBlogId)->willReturn($activeProfile); $siteHelper = $this->createMock(SiteHelper::class); @@ -251,6 +243,7 @@ function (ConfigurationProfileEntity $configurationProfile) use ($activeProfile) $submission = $this->createMock(SubmissionEntity::class); $submission->method('getId')->willReturn(17); + $submission->expects(self::never())->method('setConfigurationProfileId'); $submissionManager = $this->getMockBuilder(SubmissionManager::class)->disableOriginalConstructor()->getMock(); $submissionManager->method('findOne')->willReturn($submission); @@ -262,43 +255,21 @@ function (ConfigurationProfileEntity $configurationProfile) use ($activeProfile) $x = $this->getContentRelationDiscoveryService($apiWrapper, $contentHelper, $settingsManager, $submissionManager, wpProxy: $wpProxy); $x->createSubmissions(UserTranslationRequest::fromArray([ - 'source' => ['contentType' => $contentType, 'id' => [$sourceId]], + 'source' => ['contentType' => 'post', 'id' => [$sourceId]], 'job' => [ - 'id' => $jobUid, - 'name' => $jobName, + 'id' => 'abcdef123456', + 'name' => 'Job Name', 'description' => '', 'dueDate' => '', 'timeZone' => 'Europe/Kiev', 'authorize' => 'true', ], - 'targetBlogIds' => $targetBlogId, + 'targetBlogIds' => 2, 'relations' => [], - 'profileId' => $requestedProfileId, ])); } - public static function invalidRequestedProfileProvider(): array - { - $foreignBlogLocale = new Locale(); - $foreignBlogLocale->setBlogId(99); // not the current blog (1) - $foreignBlogProfile = new ConfigurationProfileEntity(); - $foreignBlogProfile->setSourceLocale($foreignBlogLocale); - $foreignBlogProfile->setIsActive(1); - - $sameBlogLocale = new Locale(); - $sameBlogLocale->setBlogId(1); - $inactiveProfile = new ConfigurationProfileEntity(); - $inactiveProfile->setSourceLocale($sameBlogLocale); - $inactiveProfile->setIsActive(0); - - return [ - 'profile does not exist' => [null], - 'profile belongs to a different blog' => [$foreignBlogProfile], - 'profile is not active' => [$inactiveProfile], - ]; - } - public function testCreateSubmissionsUsesRequestedProfileWhenValidAndActive() { $sourceBlogId = 1; @@ -324,7 +295,7 @@ function (ConfigurationProfileEntity $configurationProfile) use ($requestedProfi ); $settingsManager = $this->createMock(SettingsManager::class); - $settingsManager->method('getEntityById')->with($requestedProfileId)->willReturn([$requestedProfile]); + $settingsManager->method('resolveRequestedProfile')->with($requestedProfileId, $sourceBlogId)->willReturn($requestedProfile); $settingsManager->expects(self::never())->method('getSingleSettingsProfile'); $siteHelper = $this->createMock(SiteHelper::class); @@ -538,7 +509,7 @@ public function testBulkSubmitHandlerStampsConfigurationProfile() $profile->method('getId')->willReturn($profileId); $settingsManager = $this->createMock(SettingsManager::class); - $settingsManager->method('getSingleSettingsProfile')->willReturn($profile); + $settingsManager->method('resolveRequestedProfile')->with(5, $sourceBlogId)->willReturn($profile); $siteHelper = $this->createMock(SiteHelper::class); $siteHelper->method('getCurrentBlogId')->willReturn($sourceBlogId); diff --git a/tests/Smartling/Settings/SettingsManagerTest.php b/tests/Smartling/Settings/SettingsManagerTest.php index 1fb5c201b..1d5dd2552 100644 --- a/tests/Smartling/Settings/SettingsManagerTest.php +++ b/tests/Smartling/Settings/SettingsManagerTest.php @@ -178,6 +178,88 @@ public function testGetProfileBySubmissionFallsBackWhenStoredProfileWasDeleted() self::assertSame($active, $mock->getProfileBySubmission($submission)); } + private function resolverMock(): SettingsManager + { + $mock = $this->createPartialMock(SettingsManager::class, ['getEntityById', 'findEntityByMainLocale', 'getLogger']); + $mock->method('getLogger')->willReturn(new NullLogger()); + + return $mock; + } + + private function profileForBlog(int $id, int $blogId, int $active): ConfigurationProfileEntity + { + $profile = $this->profileWithId($id); + $locale = new Locale(); + $locale->setBlogId($blogId); + $profile->setSourceLocale($locale); + $profile->setIsActive($active); + + return $profile; + } + + public function testResolveRequestedProfileUsesRequestedWhenValidAndActive() + { + $requested = $this->profileForBlog(5, 1, 1); + $mock = $this->resolverMock(); + $mock->expects(self::once())->method('getEntityById')->with(5)->willReturn([$requested]); + $mock->expects(self::never())->method('findEntityByMainLocale'); + + self::assertSame($requested, $mock->resolveRequestedProfile(5, 1)); + } + + public function testResolveRequestedProfileRejectsUnknownProfile() + { + $this->expectException(SmartlingDbException::class); + $mock = $this->resolverMock(); + $mock->method('getEntityById')->with(5)->willReturn([]); + + $mock->resolveRequestedProfile(5, 1); + } + + public function testResolveRequestedProfileRejectsProfileOfDifferentBlog() + { + $this->expectException(SmartlingDbException::class); + $mock = $this->resolverMock(); + $mock->method('getEntityById')->with(5)->willReturn([$this->profileForBlog(5, 99, 1)]); + + $mock->resolveRequestedProfile(5, 1); + } + + public function testResolveRequestedProfileRejectsInactiveProfile() + { + $this->expectException(SmartlingDbException::class); + $mock = $this->resolverMock(); + $mock->method('getEntityById')->with(5)->willReturn([$this->profileForBlog(5, 1, 0)]); + + $mock->resolveRequestedProfile(5, 1); + } + + public function testResolveRequestedProfileUsesTheOnlyActiveProfileWhenNoneRequested() + { + $only = $this->profileForBlog(3, 1, 1); + $mock = $this->resolverMock(); + $mock->expects(self::never())->method('getEntityById'); + $mock->method('findEntityByMainLocale')->with(1)->willReturn([$only]); + + self::assertSame($only, $mock->resolveRequestedProfile(null, 1)); + } + + public function testResolveRequestedProfileReturnsNullWhenNoneRequestedAndSeveralActive() + { + $mock = $this->resolverMock(); + $mock->method('findEntityByMainLocale')->with(1)->willReturn([$this->profileForBlog(3, 1, 1), $this->profileForBlog(4, 1, 1)]); + + self::assertNull($mock->resolveRequestedProfile(null, 1)); + } + + public function testResolveRequestedProfileReturnsNullWhenNoneRequestedAndNoneActive() + { + $mock = $this->resolverMock(); + $mock->method('findEntityByMainLocale')->with(1)->willReturn([]); + + self::assertNull($mock->resolveRequestedProfile(null, 1)); + } + public function testGetEntitiesQueries() { $db = $this->createMock(SmartlingToCMSDatabaseAccessWrapperInterface::class); diff --git a/tests/Smartling/WP/Controller/ContentEditJobControllerTest.php b/tests/Smartling/WP/Controller/ContentEditJobControllerTest.php deleted file mode 100644 index f46a593b5..000000000 --- a/tests/Smartling/WP/Controller/ContentEditJobControllerTest.php +++ /dev/null @@ -1,141 +0,0 @@ -createMock(ApiWrapperInterface::class), - $this->createMock(LocalizationPluginProxyInterface::class), - $this->createMock(PluginInfo::class), - $settingsManager, - $this->createMock(SiteHelper::class), - $this->getMockBuilder(SubmissionManager::class)->disableOriginalConstructor()->getMock(), - $this->createMock(Cache::class), - $this->createMock(WordpressFunctionProxyHelper::class), - ); - } - - private function profileWithId(int $id): ConfigurationProfileEntity - { - $profile = new ConfigurationProfileEntity(); - $profile->setId($id); - - return $profile; - } - - public function testResolveRequestedProfileUsesRequestedWhenValidAndActive() - { - $blogId = 1; - $requested = $this->profileWithId(5); - $sourceLocale = new Locale(); - $sourceLocale->setBlogId($blogId); - $requested->setSourceLocale($sourceLocale); - $requested->setIsActive(1); - - $settingsManager = $this->createMock(SettingsManager::class); - $settingsManager->expects(self::once())->method('getEntityById')->with(5)->willReturn([$requested]); - $settingsManager->expects(self::never())->method('getSingleSettingsProfile'); - - $result = $this->invokeMethod($this->getController($settingsManager), 'resolveRequestedProfile', [5, $blogId]); - - self::assertSame($requested, $result); - } - - public function testResolveRequestedProfileFallsBackWhenNoneRequested() - { - $blogId = 1; - $active = $this->profileWithId(3); - - $settingsManager = $this->createMock(SettingsManager::class); - $settingsManager->expects(self::never())->method('getEntityById'); - $settingsManager->expects(self::once())->method('getSingleSettingsProfile')->with($blogId)->willReturn($active); - - $result = $this->invokeMethod($this->getController($settingsManager), 'resolveRequestedProfile', [null, $blogId]); - - self::assertSame($active, $result); - } - - public function testResolveRequestedProfileFallsBackWhenRequestedNotFound() - { - $blogId = 1; - $active = $this->profileWithId(3); - - $settingsManager = $this->createMock(SettingsManager::class); - $settingsManager->method('getEntityById')->with(5)->willReturn([]); - $settingsManager->expects(self::once())->method('getSingleSettingsProfile')->with($blogId)->willReturn($active); - - $result = $this->invokeMethod($this->getController($settingsManager), 'resolveRequestedProfile', [5, $blogId]); - - self::assertSame($active, $result); - } - - public function testResolveRequestedProfileFallsBackWhenRequestedBelongsToDifferentBlog() - { - $blogId = 1; - $requested = $this->profileWithId(5); - $foreignLocale = new Locale(); - $foreignLocale->setBlogId(99); - $requested->setSourceLocale($foreignLocale); - $requested->setIsActive(1); - $active = $this->profileWithId(3); - - $settingsManager = $this->createMock(SettingsManager::class); - $settingsManager->method('getEntityById')->with(5)->willReturn([$requested]); - $settingsManager->expects(self::once())->method('getSingleSettingsProfile')->with($blogId)->willReturn($active); - - $result = $this->invokeMethod($this->getController($settingsManager), 'resolveRequestedProfile', [5, $blogId]); - - self::assertSame($active, $result); - } - - public function testResolveRequestedProfileFallsBackWhenRequestedIsInactive() - { - $blogId = 1; - $requested = $this->profileWithId(5); - $sourceLocale = new Locale(); - $sourceLocale->setBlogId($blogId); - $requested->setSourceLocale($sourceLocale); - $requested->setIsActive(0); - $active = $this->profileWithId(3); - - $settingsManager = $this->createMock(SettingsManager::class); - $settingsManager->method('getEntityById')->with(5)->willReturn([$requested]); - $settingsManager->expects(self::once())->method('getSingleSettingsProfile')->with($blogId)->willReturn($active); - - $result = $this->invokeMethod($this->getController($settingsManager), 'resolveRequestedProfile', [5, $blogId]); - - self::assertSame($active, $result); - } - - public function testResolveRequestedProfilePropagatesExceptionWhenNoActiveProfileEither() - { - $this->expectException(SmartlingDbException::class); - $blogId = 1; - - $settingsManager = $this->createMock(SettingsManager::class); - $settingsManager->method('getEntityById')->with(5)->willReturn([]); - $settingsManager->method('getSingleSettingsProfile')->with($blogId)->willThrowException(new SmartlingDbException('no active profile')); - - $this->invokeMethod($this->getController($settingsManager), 'resolveRequestedProfile', [5, $blogId]); - } -} diff --git a/tests/Smartling/WP/Controller/InstantTranslationControllerTest.php b/tests/Smartling/WP/Controller/InstantTranslationControllerTest.php index b2516ca64..0c62ac69c 100644 --- a/tests/Smartling/WP/Controller/InstantTranslationControllerTest.php +++ b/tests/Smartling/WP/Controller/InstantTranslationControllerTest.php @@ -7,6 +7,7 @@ use Smartling\Helpers\AjaxSecurityChecker; use Smartling\Helpers\FileUriHelper; use Smartling\Helpers\WordpressFunctionProxyHelper; +use Smartling\Settings\SettingsManager; use Smartling\Submissions\SubmissionEntity; use Smartling\Submissions\SubmissionFactory; use Smartling\Submissions\SubmissionManager; @@ -39,6 +40,7 @@ protected function setUp(): void $this->fileUriHelper, $this->wpProxy, $this->ajaxSecurity, + $this->createMock(SettingsManager::class), ); } @@ -149,7 +151,7 @@ public function testGetOrCreateSubmissionCreatesNew(): void // Manager should store it $this->submissionManager->method('storeEntity')->willReturn($newSubmission); - $result = $method->invoke($this->controller, 1, 2, 'post', 123); + $result = $method->invoke($this->controller, 1, 2, 'post', 123, null); $this->assertInstanceOf(SubmissionEntity::class, $result); } @@ -167,7 +169,7 @@ public function testGetOrCreateSubmissionReusesExisting(): void $this->submissionManager->method('findOne')->willReturn($existingSubmission); $this->submissionManager->method('storeEntity')->willReturn($existingSubmission); - $result = $method->invoke($this->controller, 1, 2, 'post', 123); + $result = $method->invoke($this->controller, 1, 2, 'post', 123, null); $this->assertInstanceOf(SubmissionEntity::class, $result); $this->assertSame($existingSubmission, $result); @@ -196,7 +198,8 @@ public function testBuildSubmissionsWithNoRelations(): void 123, 1, [2, 3], - [] // No relations + [], // No relations + null ); // Should create 2 submissions (1 main content × 2 target blogs) @@ -235,7 +238,8 @@ public function testBuildSubmissionsWithRelations(): void 123, 1, [2, 3], - $relations + $relations, + null ); // Should create: @@ -275,7 +279,8 @@ public function testBuildSubmissionsExcludesMainContentFromRelations(): void 123, 1, [2], - $relations + $relations, + null ); // Should create: @@ -515,4 +520,19 @@ public function testHandlePollStatusDoesNothingWhenUnauthorized(): void $this->controller->handlePollStatus(); } + + public function testGetOrCreateSubmissionStampsRequestedProfile(): void + { + $reflection = new \ReflectionClass($this->controller); + $method = $reflection->getMethod('getOrCreateSubmission'); + $method->setAccessible(true); + + $existingSubmission = $this->createMock(SubmissionEntity::class); + $existingSubmission->method('setStatus')->willReturnSelf(); + $existingSubmission->expects($this->once())->method('setConfigurationProfileId')->with(7); + $this->submissionManager->method('findOne')->willReturn($existingSubmission); + $this->submissionManager->method('storeEntity')->willReturn($existingSubmission); + + $method->invoke($this->controller, 1, 2, 'post', 123, 7); + } } From 559dc4570a49f6f0cad8d8d8f9d5e9749cbecd5e Mon Sep 17 00:00:00 2001 From: Vitalii Solovei Date: Wed, 7 Oct 2026 13:25:25 +0200 Subject: [PATCH 07/14] always stamp the resolved profile on submissions (WP-1022) Co-Authored-By: Claude Sonnet 5.5 --- .../ContentRelationsDiscoveryService.php | 23 ++++--------------- .../ContentRelationsDiscoveryServiceTest.php | 5 ++-- 2 files changed, 8 insertions(+), 20 deletions(-) diff --git a/inc/Smartling/Services/ContentRelationsDiscoveryService.php b/inc/Smartling/Services/ContentRelationsDiscoveryService.php index c02512a21..0e21d2f07 100644 --- a/inc/Smartling/Services/ContentRelationsDiscoveryService.php +++ b/inc/Smartling/Services/ContentRelationsDiscoveryService.php @@ -88,7 +88,6 @@ public function bulkUpload( ConfigurationProfileEntity $profile, array $targetBlogIds, bool $enqueue = true, - bool $stampProfile = true, ): array { $this->getLogger()->debug("Bulk upload request, contentIds=" . implode(',', $contentIds)); $queueIds = []; @@ -96,8 +95,7 @@ public function bulkUpload( foreach ($targetBlogIds as $targetBlogId) { foreach ($contentIds as $id) { $submission = $this->submissionManager->findTargetBlogSubmission($contentType, $currentBlogId, $id, $targetBlogId); - $isNew = $submission === null; - if ($isNew) { + if ($submission === null) { $submission = $this->submissionManager->getSubmissionEntity($contentType, $currentBlogId, $id, $targetBlogId, $this->localizationPluginProxy); $title = $this->getTitle($submission); if ($title !== '') { @@ -109,12 +107,7 @@ public function bulkUpload( $submission->setStatus(SubmissionEntity::SUBMISSION_STATUS_NEW); $submission->setIsCloned(0); // Bulk-submitting is an explicit new translation request: (re)stamp with the profile it was requested with. - // Without a resolved profile, existing submissions keep theirs and new ones stay unstamped. - if ($stampProfile) { - $submission->setConfigurationProfileId($profile->getId()); - } elseif ($isNew) { - $submission->setConfigurationProfileId(null); - } + $submission->setConfigurationProfileId($profile->getId()); $submission = $this->submissionManager->storeEntity($submission); $queueIds[] = $submission->getId(); $this->logSubmissionCreated($submission, 'Bulk upload request', $jobInfo); @@ -137,7 +130,6 @@ public function bulkUpload( $profile, $targetBlogIds, false, - $stampProfile, )); } if ($enqueue) { @@ -159,7 +151,6 @@ public function createSubmissions(UserTranslationRequest $request): void { $curBlogId = $this->wordpressProxy->get_current_blog_id(); $requestedProfile = $this->settingsManager->resolveRequestedProfile($request->getProfileId(), $curBlogId); - $stampProfile = $requestedProfile !== null; $profile = $requestedProfile ?? $this->settingsManager->getSingleSettingsProfile($curBlogId); $job = $request->getJobInformation(); $jobInfo = new JobEntity($job->getName(), $job->getId(), $profile->getProjectId()); @@ -172,8 +163,6 @@ public function createSubmissions(UserTranslationRequest $request): void $jobInfo, $profile, $request->getTargetBlogIds(), - true, - $stampProfile, ); return; } @@ -240,10 +229,8 @@ public function createSubmissions(UserTranslationRequest $request): void $submission->setStatus(SubmissionEntity::SUBMISSION_STATUS_NEW); $submission->setIsCloned(0); // Resubmitting an existing submission is an explicit new translation request, so restamp it with the - // profile it was requested with. Without a resolved profile it keeps whatever it had. - if ($stampProfile) { - $submission->setConfigurationProfileId($profile->getId()); - } + // profile it was requested with. + $submission->setConfigurationProfileId($profile->getId()); $submission = $this->storeWithJobInfo($submission, $jobInfo, $request->getDescription()); $fileUris[] = $submission->getFileUri(); $queueIds[] = $submission->getId(); @@ -252,7 +239,7 @@ 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(), stamp them with the requested profile. - $submissionTemplateArray[SubmissionEntity::FIELD_CONFIGURATION_PROFILE_ID] = $stampProfile ? $profile->getId() : null; + $submissionTemplateArray[SubmissionEntity::FIELD_CONFIGURATION_PROFILE_ID] = $profile->getId(); foreach ($sources as $source) { $submissionArray = array_merge($submissionTemplateArray, [ diff --git a/tests/Services/ContentRelationsDiscoveryServiceTest.php b/tests/Services/ContentRelationsDiscoveryServiceTest.php index ff2db4b21..31c37ee2c 100644 --- a/tests/Services/ContentRelationsDiscoveryServiceTest.php +++ b/tests/Services/ContentRelationsDiscoveryServiceTest.php @@ -216,13 +216,14 @@ public function testCreateSubmissionsHandlerStampsConfigurationProfileOnExisting ])); } - public function testCreateSubmissionsDoesNotStampProfileWhenNoneCouldBeResolved() + public function testCreateSubmissionsStampsFallbackProfileWhenNoneCouldBeResolved() { $sourceBlogId = 1; $sourceId = 48; $activeProfile = $this->createMock(ConfigurationProfileEntity::class); $activeProfile->method('getProjectId')->willReturn('activeProjectUid'); + $activeProfile->method('getId')->willReturn(3); $apiWrapper = $this->createMock(ApiWrapper::class); $apiWrapper->expects($this->once())->method('createAuditLogRecord')->willReturnCallback( @@ -243,7 +244,7 @@ function (ConfigurationProfileEntity $configurationProfile) use ($activeProfile) $submission = $this->createMock(SubmissionEntity::class); $submission->method('getId')->willReturn(17); - $submission->expects(self::never())->method('setConfigurationProfileId'); + $submission->expects(self::once())->method('setConfigurationProfileId')->with(3); $submissionManager = $this->getMockBuilder(SubmissionManager::class)->disableOriginalConstructor()->getMock(); $submissionManager->method('findOne')->willReturn($submission); From 151c34538ccdb05646ee0906a7893ec63020f8fe Mon Sep 17 00:00:00 2001 From: Vitalii Solovei Date: Wed, 7 Oct 2026 13:45:28 +0200 Subject: [PATCH 08/14] move parameter down, inline variable (WP-1022) --- inc/Smartling/Models/UserTranslationRequest.php | 15 ++++++++++++--- .../Services/ContentRelationsDiscoveryService.php | 4 ++-- inc/Smartling/WP/Controller/TestRunController.php | 4 ++-- inc/Smartling/WP/View/BulkSubmit.php | 3 ++- .../tests/SubmissionUploadTest.php | 4 ++++ 5 files changed, 22 insertions(+), 8 deletions(-) diff --git a/inc/Smartling/Models/UserTranslationRequest.php b/inc/Smartling/Models/UserTranslationRequest.php index a6afeca36..d613fc767 100644 --- a/inc/Smartling/Models/UserTranslationRequest.php +++ b/inc/Smartling/Models/UserTranslationRequest.php @@ -16,7 +16,16 @@ class UserTranslationRequest private ?int $profileId; private array $ids; - public function __construct(int $contentId, string $contentType, array $relations, array $targetBlogIds, JobInformation $jobInformation, ?int $profileId = null, array $ids = [], string $description = '') + public function __construct( + int $contentId, + string $contentType, + array $relations, + array $targetBlogIds, + JobInformation $jobInformation, + array $ids = [], + string $description = '', + ?int $profileId = null, + ) { $this->contentId = $contentId; $this->contentType = $contentType; @@ -25,8 +34,8 @@ public function __construct(int $contentId, string $contentType, array $relation $this->relations = $relations; $this->targetBlogIds = ArrayHelper::toArrayOfIntegers($targetBlogIds, 'Target blog id expected to be numeric'); $this->jobInformation = $jobInformation; - $this->profileId = $profileId; $this->ids = self::toIntegerArray($ids); + $this->profileId = $profileId; } public function getContentId(): int @@ -84,9 +93,9 @@ public static function fromArray(array $array): self $array['relations'] ?? [], explode(',', $array['targetBlogIds']), new JobInformation($array['job']['id'], $array['job']['authorize'] === 'true', $array['job']['name'], $array['job']['description'], $array['job']['dueDate'], $array['job']['timeZone']), - self::parseProfileId($array['profileId'] ?? null), $ids, $array['description'] ?? (count($ids) > 0 ? 'From Bulk Submit' : 'From Widget'), + self::parseProfileId($array['profileId'] ?? null), ); } diff --git a/inc/Smartling/Services/ContentRelationsDiscoveryService.php b/inc/Smartling/Services/ContentRelationsDiscoveryService.php index 0e21d2f07..d9270fadb 100644 --- a/inc/Smartling/Services/ContentRelationsDiscoveryService.php +++ b/inc/Smartling/Services/ContentRelationsDiscoveryService.php @@ -150,8 +150,8 @@ public function bulkUpload( public function createSubmissions(UserTranslationRequest $request): void { $curBlogId = $this->wordpressProxy->get_current_blog_id(); - $requestedProfile = $this->settingsManager->resolveRequestedProfile($request->getProfileId(), $curBlogId); - $profile = $requestedProfile ?? $this->settingsManager->getSingleSettingsProfile($curBlogId); + $profile = $this->settingsManager->resolveRequestedProfile($request->getProfileId(), $curBlogId) ?? + $this->settingsManager->getSingleSettingsProfile($curBlogId); $job = $request->getJobInformation(); $jobInfo = new JobEntity($job->getName(), $job->getId(), $profile->getProjectId()); diff --git a/inc/Smartling/WP/Controller/TestRunController.php b/inc/Smartling/WP/Controller/TestRunController.php index 36e10e078..a4a7fb0ed 100644 --- a/inc/Smartling/WP/Controller/TestRunController.php +++ b/inc/Smartling/WP/Controller/TestRunController.php @@ -199,9 +199,9 @@ public function testRun($data): void ->getRelations($post->post_type, $post->ID, [$targetBlogId])->getReferences()], [$targetBlogId], new JobInformation($job->getJobUid(), true, $job->getJobName(), 'Test run job', '', 'UTC'), - $profile->getId(), [], - 'Test run' + 'Test run', + $profile->getId(), )); } diff --git a/inc/Smartling/WP/View/BulkSubmit.php b/inc/Smartling/WP/View/BulkSubmit.php index c6290c4a1..081a04785 100644 --- a/inc/Smartling/WP/View/BulkSubmit.php +++ b/inc/Smartling/WP/View/BulkSubmit.php @@ -1,6 +1,7 @@ display() ?>
$p->toWizardArray(), $data->getApplicableProfiles()); + $profilesData = array_map(static fn(ConfigurationProfileEntity $p) => $p->toWizardArray(), $data->getApplicableProfiles()); ?>
getId(), )); $submissions = $submissionManager->find([SubmissionEntity::FIELD_SOURCE_ID => $postId]); @@ -89,6 +91,8 @@ public function testUploadAttachment() [2 => ['attachment' => [$attachmentId]]], $targetBlogs, new JobInformation($job['translationJobUid'], false, $jobName, '', '', ''), + [], + '', $profile->getId(), )); $this->assertCount($existingSubmissionCount + 2, $submissionManager->find([1 => 1])); From e2fd2ba7505338f8f0cc2a7da7053c9379a9b849 Mon Sep 17 00:00:00 2001 From: Vitalii Solovei Date: Wed, 7 Oct 2026 17:04:27 +0200 Subject: [PATCH 09/14] unify profile resolution, fix job list race, hide profile errors from client (WP-1022) Co-Authored-By: Claude Sonnet 5.5 --- .../ContentRelationsDiscoveryService.php | 3 +- .../Services/ContentRelationsHandler.php | 3 + inc/Smartling/Settings/SettingsManager.php | 41 ++++++-------- .../Controller/ContentEditJobController.php | 5 +- .../InstantTranslationController.php | 13 ++--- js/app.js | 55 +++++++++++-------- .../ContentRelationsDiscoveryServiceTest.php | 13 ++--- .../Settings/SettingsManagerTest.php | 15 ++--- .../InstantTranslationControllerTest.php | 10 ++-- tests/playwright/job-wizard.spec.js | 15 +++-- 10 files changed, 84 insertions(+), 89 deletions(-) diff --git a/inc/Smartling/Services/ContentRelationsDiscoveryService.php b/inc/Smartling/Services/ContentRelationsDiscoveryService.php index d9270fadb..470a40f48 100644 --- a/inc/Smartling/Services/ContentRelationsDiscoveryService.php +++ b/inc/Smartling/Services/ContentRelationsDiscoveryService.php @@ -150,8 +150,7 @@ public function bulkUpload( public function createSubmissions(UserTranslationRequest $request): void { $curBlogId = $this->wordpressProxy->get_current_blog_id(); - $profile = $this->settingsManager->resolveRequestedProfile($request->getProfileId(), $curBlogId) ?? - $this->settingsManager->getSingleSettingsProfile($curBlogId); + $profile = $this->settingsManager->resolveRequestedProfile($request->getProfileId(), $curBlogId); $job = $request->getJobInformation(); $jobInfo = new JobEntity($job->getName(), $job->getId(), $profile->getProjectId()); diff --git a/inc/Smartling/Services/ContentRelationsHandler.php b/inc/Smartling/Services/ContentRelationsHandler.php index c73ae87a2..f67928f5f 100644 --- a/inc/Smartling/Services/ContentRelationsHandler.php +++ b/inc/Smartling/Services/ContentRelationsHandler.php @@ -3,6 +3,7 @@ namespace Smartling\Services; use Exception; +use Smartling\Exception\SmartlingDbException; use Smartling\Exception\SmartlingHumanReadableException; use Smartling\Helpers\AjaxAuthorizationFailure; use Smartling\Helpers\AjaxSecurityChecker; @@ -101,6 +102,8 @@ public function createSubmissionsHandler(array $data = null): void try { $this->service->createSubmissions(UserTranslationRequest::fromArray($data)); $this->returnResponse(['status' => BaseAjaxServiceAbstract::RESPONSE_SUCCESS]); + } catch (SmartlingDbException $e) { + $this->returnError('content.submission.failed', 'Invalid translation profile'); } catch (Exception $e) { $this->returnError('content.submission.failed', $e->getMessage()); } diff --git a/inc/Smartling/Settings/SettingsManager.php b/inc/Smartling/Settings/SettingsManager.php index 40b17e22e..9b70eca5b 100644 --- a/inc/Smartling/Settings/SettingsManager.php +++ b/inc/Smartling/Settings/SettingsManager.php @@ -122,39 +122,30 @@ public function getSingleSettingsProfile(int $mainBlogId): ConfigurationProfileE /** * Resolves the profile for a translation request. * An explicitly requested profile must exist, belong to the blog and be active, otherwise the request is rejected. - * Without an explicit profile, the blog's only active profile is used; with none or several, null is returned. + * Without an explicit profile, the blog's active profile is used. * * @throws SmartlingDbException */ - public function resolveRequestedProfile(?int $requestedProfileId, int $blogId): ?ConfigurationProfileEntity + public function resolveRequestedProfile(?int $requestedProfileId, int $blogId): ConfigurationProfileEntity { - if ($requestedProfileId !== null) { - $profile = ArrayHelper::first($this->getEntityById($requestedProfileId)); - if (!$profile instanceof ConfigurationProfileEntity) { - $message = "Requested profileId=$requestedProfileId not found"; - $this->getLogger()->warning($message); - throw new SmartlingDbException($message); - } - if ($profile->getSourceLocale()->getBlogId() !== $blogId || 1 !== $profile->getIsActive()) { - $message = "Requested profileId=$requestedProfileId is not an active profile for blogId=$blogId"; - $this->getLogger()->warning($message); - throw new SmartlingDbException($message); - } - $this->getLogger()->debug("Using requested profileId=$requestedProfileId for blogId=$blogId"); - - return $profile; + if ($requestedProfileId === null) { + return $this->getSingleSettingsProfile($blogId); } - $profiles = $this->findEntityByMainLocale($blogId); - if (1 === count($profiles)) { - $profile = ArrayHelper::first($profiles); - $this->getLogger()->debug("No profile requested, using the only active profileId={$profile->getId()} for blogId=$blogId"); - - return $profile; + $profile = ArrayHelper::first($this->getEntityById($requestedProfileId)); + if (!$profile instanceof ConfigurationProfileEntity) { + $message = "Requested profileId=$requestedProfileId not found"; + $this->getLogger()->warning($message); + throw new SmartlingDbException($message); + } + if ($profile->getSourceLocale()->getBlogId() !== $blogId || 1 !== $profile->getIsActive()) { + $message = "Requested profileId=$requestedProfileId is not an active profile for blogId=$blogId"; + $this->getLogger()->warning($message); + throw new SmartlingDbException($message); } - $this->getLogger()->debug('No profile requested and ' . count($profiles) . " active profiles found for blogId=$blogId, profile will not be stored"); + $this->getLogger()->debug("Using requested profileId=$requestedProfileId for blogId=$blogId"); - return null; + return $profile; } /** diff --git a/inc/Smartling/WP/Controller/ContentEditJobController.php b/inc/Smartling/WP/Controller/ContentEditJobController.php index af92547e2..73c090032 100644 --- a/inc/Smartling/WP/Controller/ContentEditJobController.php +++ b/inc/Smartling/WP/Controller/ContentEditJobController.php @@ -109,9 +109,10 @@ public function initJobApiProxy(): void $profile = $this->settingsManager->resolveRequestedProfile( UserTranslationRequest::parseProfileId($params['profileId'] ?? null), $blogId, - ) ?? $this->settingsManager->getSingleSettingsProfile($blogId); + ); } catch (\InvalidArgumentException | SmartlingDbException $e) { - $this->wpProxy->wp_send_json(['status' => 400, 'message' => ['profileId' => $e->getMessage()]], 400); + $this->getLogger()->warning('Unable to resolve requested profile: ' . $e->getMessage()); + $this->wpProxy->wp_send_json(['status' => 400, 'message' => ['profileId' => 'Invalid translation profile']], 400); return; } diff --git a/inc/Smartling/WP/Controller/InstantTranslationController.php b/inc/Smartling/WP/Controller/InstantTranslationController.php index 6066dc9ea..69e3c2523 100644 --- a/inc/Smartling/WP/Controller/InstantTranslationController.php +++ b/inc/Smartling/WP/Controller/InstantTranslationController.php @@ -76,9 +76,10 @@ public function handleRequestTranslation(): void $profileId = $this->settingsManager->resolveRequestedProfile( UserTranslationRequest::parseProfileId($_POST['profileId'] ?? null), $sourceBlogId, - )?->getId(); + )->getId(); } catch (\InvalidArgumentException | SmartlingDbException $e) { - $this->wpProxy->wp_send_json_error(['message' => $e->getMessage()], 400); + $this->getLogger()->warning('Unable to resolve requested profile: ' . $e->getMessage()); + $this->wpProxy->wp_send_json_error(['message' => 'Invalid translation profile'], 400); return; } @@ -224,7 +225,7 @@ private function buildSubmissions( int $sourceBlogId, array $targetBlogIds, array $relations, - ?int $profileId, + int $profileId, ): array { $submissions = []; @@ -294,7 +295,7 @@ private function getOrCreateSubmission( int $targetBlogId, string $contentType, int $contentId, - ?int $profileId, + int $profileId, ): ?SubmissionEntity { try { $submission = $this->submissionManager->findOne([ @@ -319,9 +320,7 @@ private function getOrCreateSubmission( $submission->setStatus(SubmissionEntity::SUBMISSION_STATUS_NEW); } - if ($profileId !== null) { - $submission->setConfigurationProfileId($profileId); - } + $submission->setConfigurationProfileId($profileId); $submission->setStatus(SubmissionEntity::SUBMISSION_STATUS_IN_PROGRESS); return $this->submissionManager->storeEntity($submission); } catch (\Exception $e) { diff --git a/js/app.js b/js/app.js index f1b7e1ec9..94c3344a1 100644 --- a/js/app.js +++ b/js/app.js @@ -92,27 +92,34 @@ function JobWizard({ isBulkSubmitPage, contentType, contentId, profiles, blogId, const [instantSubmissionIds, setInstantSubmissionIds] = useState([]); const [instantCompletedCount, setInstantCompletedCount] = useState(0); - const loadJobs = useCallback(async () => { - try { - const response = await jQuery.post(adminUrl, { - action: 'smartling_job_api_proxy', - _wpnonce: nonce, - innerAction: 'list-jobs', - params: { profileId: selectedProfileId } - }); - if (response.status === 200) { - setJobs(response.data); - } - } catch (e) { - setError('Failed to load jobs'); - } finally { - setLoading(false); - } - }, [adminUrl, selectedProfileId]); - useEffect(() => { - loadJobs(); - }, [loadJobs]); + // Ignore responses for a previously selected profile, a slow one must not overwrite the current job list + let stale = false; + (async () => { + try { + const response = await jQuery.post(adminUrl, { + action: 'smartling_job_api_proxy', + _wpnonce: nonce, + innerAction: 'list-jobs', + params: { profileId: selectedProfileId } + }); + if (!stale && response.status === 200) { + setJobs(response.data); + } + } catch (e) { + if (!stale) { + setError('Failed to load jobs'); + } + } finally { + if (!stale) { + setLoading(false); + } + } + })(); + return () => { + stale = true; + }; + }, [adminUrl, nonce, selectedProfileId]); const handleProfileChange = (val) => { const newProfileId = parseInt(val, 10); @@ -156,7 +163,7 @@ function JobWizard({ isBulkSubmitPage, contentType, contentId, profiles, blogId, if (isBulkSubmitPage) { jQuery('input.bulkaction[type=checkbox]:checked').each(function() { const parts = jQuery(this).attr('id').split('-'); - const id = parseInt(parts.shift()); + const id = parseInt(parts.shift(), 10); const type = parts.join('-'); loadRelations(type, id, 1); }); @@ -356,7 +363,7 @@ function JobWizard({ isBulkSubmitPage, contentType, contentId, profiles, blogId, data.ids = []; jQuery('input.bulkaction[type=checkbox]:checked').each(function() { const parts = jQuery(this).attr('id').split('-'); - data.ids.push(parseInt(parts.shift())); + data.ids.push(parseInt(parts.shift(), 10)); data.source.contentType = parts.join('-'); }); } @@ -597,9 +604,9 @@ if (document.getElementById('smartling-app')) { const container = document.getElementById('smartling-app'); const isBulkSubmitPage = container.dataset.bulkSubmit === 'true'; const contentType = container.dataset.contentType || ''; - const contentId = parseInt(container.dataset.contentId) || 0; + const contentId = parseInt(container.dataset.contentId, 10) || 0; const profiles = JSON.parse(container.dataset.profiles || '[]'); - const blogId = parseInt(container.dataset.blogId) || 0; + const blogId = parseInt(container.dataset.blogId, 10) || 0; const ajaxUrl = container.dataset.ajaxUrl || ''; const adminUrl = container.dataset.adminUrl || ''; const nonce = container.dataset.nonce || ''; diff --git a/tests/Services/ContentRelationsDiscoveryServiceTest.php b/tests/Services/ContentRelationsDiscoveryServiceTest.php index 31c37ee2c..6875ff4de 100644 --- a/tests/Services/ContentRelationsDiscoveryServiceTest.php +++ b/tests/Services/ContentRelationsDiscoveryServiceTest.php @@ -110,7 +110,7 @@ public function testCreateSubmissionsHandler() }); $settingsManager = $this->createMock(SettingsManager::class); - $settingsManager->method('getSingleSettingsProfile')->willReturn($profile); + $settingsManager->method('resolveRequestedProfile')->willReturn($profile); $siteHelper = $this->createMock(SiteHelper::class); $siteHelper->method('getCurrentBlogId')->willReturn($sourceBlogId); @@ -233,8 +233,7 @@ function (ConfigurationProfileEntity $configurationProfile) use ($activeProfile) ); $settingsManager = $this->createMock(SettingsManager::class); - $settingsManager->method('resolveRequestedProfile')->with(null, $sourceBlogId)->willReturn(null); - $settingsManager->expects(self::once())->method('getSingleSettingsProfile')->with($sourceBlogId)->willReturn($activeProfile); + $settingsManager->method('resolveRequestedProfile')->with(null, $sourceBlogId)->willReturn($activeProfile); $siteHelper = $this->createMock(SiteHelper::class); $siteHelper->method('getCurrentBlogId')->willReturn($sourceBlogId); @@ -413,7 +412,7 @@ public function testBulkSubmitHandler() $profile->method('getProjectId')->willReturn($projectUid); $settingsManager = $this->createMock(SettingsManager::class); - $settingsManager->method('getSingleSettingsProfile')->willReturn($profile); + $settingsManager->method('resolveRequestedProfile')->willReturn($profile); $siteHelper = $this->createMock(SiteHelper::class); $siteHelper->method('getCurrentBlogId')->willReturn($sourceBlogId); @@ -592,7 +591,7 @@ public function testExistingMenuItemsGetSubmittedOnExistingMenuBulkSubmit() $profile->method('getProjectId')->willReturn($projectUid); $settingsManager = $this->createMock(SettingsManager::class); - $settingsManager->method('getSingleSettingsProfile')->willReturn($profile); + $settingsManager->method('resolveRequestedProfile')->willReturn($profile); $siteHelper = $this->createMock(SiteHelper::class); $siteHelper->method('getCurrentBlogId')->willReturn($sourceBlogId); @@ -685,7 +684,7 @@ public function testJobInfoGetsStoredOnNewSubmissions() $profile->method('getProjectId')->willReturn($projectUid); $settingsManager = $this->createMock(SettingsManager::class); - $settingsManager->method('getSingleSettingsProfile')->willReturn($profile); + $settingsManager->method('resolveRequestedProfile')->willReturn($profile); $siteHelper = $this->createMock(SiteHelper::class); $siteHelper->method('getCurrentBlogId')->willReturn($sourceBlogId); @@ -868,7 +867,7 @@ public function testRelatedItemsSentForTranslation() $profile->method('getProjectId')->willReturn($projectUid); $settingsManager = $this->createMock(SettingsManager::class); - $settingsManager->method('getSingleSettingsProfile')->willReturn($profile); + $settingsManager->method('resolveRequestedProfile')->willReturn($profile); $siteHelper = $this->createMock(SiteHelper::class); $siteHelper->method('getCurrentBlogId')->willReturn($sourceBlogId); diff --git a/tests/Smartling/Settings/SettingsManagerTest.php b/tests/Smartling/Settings/SettingsManagerTest.php index 1d5dd2552..330b797a9 100644 --- a/tests/Smartling/Settings/SettingsManagerTest.php +++ b/tests/Smartling/Settings/SettingsManagerTest.php @@ -234,7 +234,7 @@ public function testResolveRequestedProfileRejectsInactiveProfile() $mock->resolveRequestedProfile(5, 1); } - public function testResolveRequestedProfileUsesTheOnlyActiveProfileWhenNoneRequested() + public function testResolveRequestedProfileFallsBackToActiveProfileWhenNoneRequested() { $only = $this->profileForBlog(3, 1, 1); $mock = $this->resolverMock(); @@ -244,20 +244,13 @@ public function testResolveRequestedProfileUsesTheOnlyActiveProfileWhenNoneReque self::assertSame($only, $mock->resolveRequestedProfile(null, 1)); } - public function testResolveRequestedProfileReturnsNullWhenNoneRequestedAndSeveralActive() - { - $mock = $this->resolverMock(); - $mock->method('findEntityByMainLocale')->with(1)->willReturn([$this->profileForBlog(3, 1, 1), $this->profileForBlog(4, 1, 1)]); - - self::assertNull($mock->resolveRequestedProfile(null, 1)); - } - - public function testResolveRequestedProfileReturnsNullWhenNoneRequestedAndNoneActive() + public function testResolveRequestedProfileThrowsWhenNoneRequestedAndNoneActive() { + $this->expectException(SmartlingDbException::class); $mock = $this->resolverMock(); $mock->method('findEntityByMainLocale')->with(1)->willReturn([]); - self::assertNull($mock->resolveRequestedProfile(null, 1)); + $mock->resolveRequestedProfile(null, 1); } public function testGetEntitiesQueries() diff --git a/tests/Smartling/WP/Controller/InstantTranslationControllerTest.php b/tests/Smartling/WP/Controller/InstantTranslationControllerTest.php index 0c62ac69c..7b7254f10 100644 --- a/tests/Smartling/WP/Controller/InstantTranslationControllerTest.php +++ b/tests/Smartling/WP/Controller/InstantTranslationControllerTest.php @@ -151,7 +151,7 @@ public function testGetOrCreateSubmissionCreatesNew(): void // Manager should store it $this->submissionManager->method('storeEntity')->willReturn($newSubmission); - $result = $method->invoke($this->controller, 1, 2, 'post', 123, null); + $result = $method->invoke($this->controller, 1, 2, 'post', 123, 7); $this->assertInstanceOf(SubmissionEntity::class, $result); } @@ -169,7 +169,7 @@ public function testGetOrCreateSubmissionReusesExisting(): void $this->submissionManager->method('findOne')->willReturn($existingSubmission); $this->submissionManager->method('storeEntity')->willReturn($existingSubmission); - $result = $method->invoke($this->controller, 1, 2, 'post', 123, null); + $result = $method->invoke($this->controller, 1, 2, 'post', 123, 7); $this->assertInstanceOf(SubmissionEntity::class, $result); $this->assertSame($existingSubmission, $result); @@ -199,7 +199,7 @@ public function testBuildSubmissionsWithNoRelations(): void 1, [2, 3], [], // No relations - null + 7 ); // Should create 2 submissions (1 main content × 2 target blogs) @@ -239,7 +239,7 @@ public function testBuildSubmissionsWithRelations(): void 1, [2, 3], $relations, - null + 7 ); // Should create: @@ -280,7 +280,7 @@ public function testBuildSubmissionsExcludesMainContentFromRelations(): void 1, [2], $relations, - null + 7 ); // Should create: diff --git a/tests/playwright/job-wizard.spec.js b/tests/playwright/job-wizard.spec.js index 2a580027d..50b219da7 100644 --- a/tests/playwright/job-wizard.spec.js +++ b/tests/playwright/job-wizard.spec.js @@ -19,16 +19,19 @@ test.describe('Job wizard — post edit page', () => { expect(nonce.length, 'data-nonce must be at least 8 characters').toBeGreaterThanOrEqual(8); }); - test('#smartling-app has valid JSON in data-locales', async ({ page }) => { + test('#smartling-app has valid JSON in data-profiles', async ({ page }) => { await page.goto(`/wp-admin/post.php?post=${POST_ID}&action=edit`, { waitUntil: 'commit' }); await page.waitForSelector('#smartling-app', { state: 'attached', timeout: 90000 }); - const localesRaw = await page.getAttribute('#smartling-app', 'data-locales'); - expect(localesRaw, 'data-locales attribute must be present').toBeTruthy(); + const profilesRaw = await page.getAttribute('#smartling-app', 'data-profiles'); + expect(profilesRaw, 'data-profiles attribute must be present').toBeTruthy(); - let locales; - expect(() => { locales = JSON.parse(localesRaw); }, 'data-locales must be valid JSON').not.toThrow(); - expect(Array.isArray(locales), 'data-locales must decode to an array').toBe(true); + let profiles; + expect(() => { profiles = JSON.parse(profilesRaw); }, 'data-profiles must be valid JSON').not.toThrow(); + expect(Array.isArray(profiles), 'data-profiles must decode to an array').toBe(true); + for (const profile of profiles) { + expect(Array.isArray(profile.locales), 'each profile must have a locales array').toBe(true); + } }); test('React job wizard renders job tabs', async ({ page }) => { From 57350490bf2da5560d4b5de37a161e512a8e7d73 Mon Sep 17 00:00:00 2001 From: Vitalii Solovei Date: Wed, 7 Oct 2026 20:46:32 +0200 Subject: [PATCH 10/14] use promoted properties (WP-1022) --- .../Models/UserTranslationRequest.php | 20 +++++-------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/inc/Smartling/Models/UserTranslationRequest.php b/inc/Smartling/Models/UserTranslationRequest.php index d613fc767..a7c71680d 100644 --- a/inc/Smartling/Models/UserTranslationRequest.php +++ b/inc/Smartling/Models/UserTranslationRequest.php @@ -7,35 +7,25 @@ class UserTranslationRequest { - private int $contentId; - private string $contentType; - private string $description; private array $relations; private array $targetBlogIds; - private JobInformation $jobInformation; - private ?int $profileId; private array $ids; public function __construct( - int $contentId, - string $contentType, + private int $contentId, + private string $contentType, array $relations, array $targetBlogIds, - JobInformation $jobInformation, + private JobInformation $jobInformation, array $ids = [], - string $description = '', - ?int $profileId = null, + private string $description = '', + private ?int $profileId = null, ) { - $this->contentId = $contentId; - $this->contentType = $contentType; - $this->description = $description; krsort($relations); $this->relations = $relations; $this->targetBlogIds = ArrayHelper::toArrayOfIntegers($targetBlogIds, 'Target blog id expected to be numeric'); - $this->jobInformation = $jobInformation; $this->ids = self::toIntegerArray($ids); - $this->profileId = $profileId; } public function getContentId(): int From 5847c8de7211ee2c1c7063cb1b5f0287f9de445c Mon Sep 17 00:00:00 2001 From: Vitalii Solovei Date: Thu, 8 Oct 2026 19:13:41 +0200 Subject: [PATCH 11/14] address review: profile resolution, validation, exception mapping (WP-1022) Several bugs surfaced once more than one profile can be active for a blog: - Related content created while processing a submission (referenced posts/terms, images) always got the source blog's default active profile via SubmissionManager::getSubmissionEntity(), even when the parent submission had been explicitly stamped with a different, user-requested one - silently sending it to the wrong Smartling project. getSubmissionEntity() now only falls back to the default when nothing is stamped yet, and an explicit profile id (the parent's) always wins. Threaded through TranslationHelper/ReferencedContentProcessor/SmartlingCoreExportApi and the legacy per-post widget's upload handler, which also now picks the profile that actually covers the selected target blogs instead of always using the first active one. - Target blog ids were never checked against the resolved profile's enabled locales in createSubmissions(), create-job, instant translation, or the legacy widget - a blog outside the profile produced an empty Smartling locale and failed later, away from the request. SettingsManager::assertTargetBlogIdsBelongToProfile() rejects this up front with a 400. - SettingsManager::resolveRequestedProfile() now throws SmartlingHumanReadableException instead of the broader SmartlingDbException, so ContentRelationsHandler can map profile resolution failures to a clean 400 without mislabeling unrelated DB errors as invalid profile errors. - Resubmitting a submission that is still IN_PROGRESS under a different profile now logs a warning, since it loses its link to the previous profile's file/job. Co-Authored-By: Claude Sonnet 5 --- inc/Smartling/Base/SmartlingCoreExportApi.php | 5 +- .../ReferencedContentProcessor.php | 1 + inc/Smartling/Helpers/TranslationHelper.php | 21 +++-- .../ContentRelationsDiscoveryService.php | 24 +++++ .../Services/ContentRelationsHandler.php | 5 +- inc/Smartling/Settings/SettingsManager.php | 42 ++++++++- .../Submissions/SubmissionManager.php | 16 +++- .../Controller/ContentEditJobController.php | 16 +++- .../InstantTranslationController.php | 21 ++++- .../PostBasedWidgetControllerStd.php | 44 ++++++++- inc/Smartling/WP/View/ContentEditJob.php | 1 + .../WP/View/post-based-content-type.php | 3 +- .../Services/ContentRelationsHandlerTest.php | 71 +++++++++++++++ .../ReferencedContentProcessorTest.php | 90 +++++++++++++++++++ .../Settings/SettingsManagerTest.php | 50 ++++++++++- .../Submissions/SubmissionManagerTest.php | 15 +++- 16 files changed, 388 insertions(+), 37 deletions(-) create mode 100644 tests/Smartling/Helpers/MetaFieldProcessor/ReferencedContentProcessorTest.php diff --git a/inc/Smartling/Base/SmartlingCoreExportApi.php b/inc/Smartling/Base/SmartlingCoreExportApi.php index 22cc5c44c..27ab9b2ce 100644 --- a/inc/Smartling/Base/SmartlingCoreExportApi.php +++ b/inc/Smartling/Base/SmartlingCoreExportApi.php @@ -28,7 +28,7 @@ public function getFullyRelateAttachmentPathByBlogId($blogId, $foundRelativePath return trim(str_replace($this->getUploadPathForSite($blogId), '', $foundRelativePath), '/'); } - public function sendAttachmentForTranslation(int $sourceBlogId, int $targetBlogId, int $sourceId, JobEntityWithBatchUid $jobInfo, bool $clone = false): SubmissionEntity + public function sendAttachmentForTranslation(int $sourceBlogId, int $targetBlogId, int $sourceId, JobEntityWithBatchUid $jobInfo, bool $clone = false, ?int $profileId = null): SubmissionEntity { return $this->getTranslationHelper()->tryPrepareRelatedContent( 'attachment', @@ -36,7 +36,8 @@ public function sendAttachmentForTranslation(int $sourceBlogId, int $targetBlogI $sourceId, $targetBlogId, $jobInfo, - $clone + $clone, + $profileId, ); } diff --git a/inc/Smartling/Helpers/MetaFieldProcessor/ReferencedContentProcessor.php b/inc/Smartling/Helpers/MetaFieldProcessor/ReferencedContentProcessor.php index cb72f78b2..28ec18c61 100644 --- a/inc/Smartling/Helpers/MetaFieldProcessor/ReferencedContentProcessor.php +++ b/inc/Smartling/Helpers/MetaFieldProcessor/ReferencedContentProcessor.php @@ -96,6 +96,7 @@ public function processFieldPreTranslation( $targetBlogId, JobEntityWithBatchUid::fromJob($submission->getJobInfo(), ''), $submission->isCloned(), + $submission->getConfigurationProfileId(), ); } diff --git a/inc/Smartling/Helpers/TranslationHelper.php b/inc/Smartling/Helpers/TranslationHelper.php index e0d156e7d..9ec7f2428 100644 --- a/inc/Smartling/Helpers/TranslationHelper.php +++ b/inc/Smartling/Helpers/TranslationHelper.php @@ -86,7 +86,7 @@ private function validateBlogs(int $sourceBlogId, int $targetBlogId): void } } - public function prepareSubmissionEntity(string $contentType, int $sourceBlog, int $sourceEntity, int $targetBlog, ?int $targetEntity = null): SubmissionEntity + public function prepareSubmissionEntity(string $contentType, int $sourceBlog, int $sourceEntity, int $targetBlog, ?int $targetEntity = null, ?int $profileId = null): SubmissionEntity { $this->validateBlogs($sourceBlog, $targetBlog); @@ -96,7 +96,8 @@ public function prepareSubmissionEntity(string $contentType, int $sourceBlog, in $sourceEntity, $targetBlog, $this->multilangProxy, - $targetEntity + $targetEntity, + $profileId, ); } @@ -107,7 +108,7 @@ public function prepareSubmissionEntity(string $contentType, int $sourceBlog, in * @throws SmartlingDataReadException * @throws SmartlingInvalidFactoryArgumentException */ - public function prepareSubmission(string $contentType, int $sourceBlog, int $sourceId, int $targetBlog, bool $clone = false): SubmissionEntity + public function prepareSubmission(string $contentType, int $sourceBlog, int $sourceId, int $targetBlog, bool $clone = false, ?int $profileId = null): SubmissionEntity { if (0 === $sourceId) { throw new \InvalidArgumentException('Source id cannot be 0.'); @@ -116,7 +117,9 @@ public function prepareSubmission(string $contentType, int $sourceBlog, int $sou $contentType, $sourceBlog, $sourceId, - $targetBlog + $targetBlog, + null, + $profileId, ); if ($submission->getFileUri() === '') { $submission->setFileUri($this->fileUriHelper->generateFileUri($submission)); @@ -158,11 +161,11 @@ public function isRelatedSubmissionCreationNeeded(string $contentType, int $sour /** * @throws SmartlingDataReadException */ - public function getExistingSubmissionOrCreateNew(string $contentType, int $sourceBlogId, int $contentId, int $targetBlogId, JobEntityWithBatchUid $jobInfo): SubmissionEntity { - $submission = $this->submissionManager->getSubmissionEntity($contentType, $sourceBlogId, $contentId, $targetBlogId, $this->multilangProxy); + public function getExistingSubmissionOrCreateNew(string $contentType, int $sourceBlogId, int $contentId, int $targetBlogId, JobEntityWithBatchUid $jobInfo, ?int $profileId = null): SubmissionEntity { + $submission = $this->submissionManager->getSubmissionEntity($contentType, $sourceBlogId, $contentId, $targetBlogId, $this->multilangProxy, null, $profileId); if ($submission->getTargetId() === 0) { $this->getLogger()->debug("Got submission with 0 target id"); - $submission = $this->tryPrepareRelatedContent($contentType, $sourceBlogId, $contentId, $targetBlogId, $jobInfo); + $submission = $this->tryPrepareRelatedContent($contentType, $sourceBlogId, $contentId, $targetBlogId, $jobInfo, false, $profileId); } return $submission; } @@ -174,9 +177,9 @@ public function getExistingSubmissionOrCreateNew(string $contentType, int $sourc * @throws SmartlingDataReadException * @throws SmartlingInvalidFactoryArgumentException */ - public function tryPrepareRelatedContent(string $contentType, int $sourceBlog, int $sourceId, int $targetBlog, JobEntityWithBatchUid $jobInfo, bool $clone = false): SubmissionEntity + public function tryPrepareRelatedContent(string $contentType, int $sourceBlog, int $sourceId, int $targetBlog, JobEntityWithBatchUid $jobInfo, bool $clone = false, ?int $profileId = null): SubmissionEntity { - $relatedSubmission = $this->prepareSubmission($contentType, $sourceBlog, $sourceId, $targetBlog, $clone); + $relatedSubmission = $this->prepareSubmission($contentType, $sourceBlog, $sourceId, $targetBlog, $clone, $profileId); if (0 !== $sourceId && 0 === $relatedSubmission->getTargetId() && SubmissionEntity::SUBMISSION_STATUS_FAILED !== $relatedSubmission->getStatus() diff --git a/inc/Smartling/Services/ContentRelationsDiscoveryService.php b/inc/Smartling/Services/ContentRelationsDiscoveryService.php index 470a40f48..11f5a4148 100644 --- a/inc/Smartling/Services/ContentRelationsDiscoveryService.php +++ b/inc/Smartling/Services/ContentRelationsDiscoveryService.php @@ -103,6 +103,7 @@ public function bulkUpload( } $submission->setFileUri($this->fileUriHelper->generateFileUri($submission)); } + $this->warnIfReprofilingInProgress($submission, $profile->getId()); $submission->setJobInfo($jobInfo); $submission->setStatus(SubmissionEntity::SUBMISSION_STATUS_NEW); $submission->setIsCloned(0); @@ -151,6 +152,7 @@ public function createSubmissions(UserTranslationRequest $request): void { $curBlogId = $this->wordpressProxy->get_current_blog_id(); $profile = $this->settingsManager->resolveRequestedProfile($request->getProfileId(), $curBlogId); + $this->settingsManager->assertTargetBlogIdsBelongToProfile($profile, $request->getTargetBlogIds()); $job = $request->getJobInformation(); $jobInfo = new JobEntity($job->getName(), $job->getId(), $profile->getProjectId()); @@ -225,6 +227,7 @@ public function createSubmissions(UserTranslationRequest $request): void 'type' => $request->getContentType(), ]; } else { + $this->warnIfReprofilingInProgress($submission, $profile->getId()); $submission->setStatus(SubmissionEntity::SUBMISSION_STATUS_NEW); $submission->setIsCloned(0); // Resubmitting an existing submission is an explicit new translation request, so restamp it with the @@ -587,6 +590,27 @@ public function normalizeReferences(array $references): array return $result; } + /** + * Re-stamping an existing submission to a different profile is an explicit, supported part of resubmitting + * under a user-chosen profile - but if it's still IN_PROGRESS under the old one, it loses its link to that + * project's file/job, and status checks/downloads will look at the new project instead. Warn so this is at + * least visible, without blocking the resubmission. + */ + private function warnIfReprofilingInProgress(SubmissionEntity $submission, int $newProfileId): void + { + $oldProfileId = $submission->getConfigurationProfileId(); + if ($oldProfileId !== null && $oldProfileId !== $newProfileId + && $submission->getStatus() === SubmissionEntity::SUBMISSION_STATUS_IN_PROGRESS + ) { + $this->getLogger()->warning(sprintf( + 'Resubmitting submissionId=%d from profileId=%d to profileId=%d while still in progress; it will lose its link to the previous profile\'s file/job.', + $submission->getId(), + $oldProfileId, + $newProfileId, + )); + } + } + private function storeWithJobInfo(SubmissionEntity $submission, JobEntity $jobInfo, string $description): SubmissionEntity { $submission->setJobInfo($jobInfo); diff --git a/inc/Smartling/Services/ContentRelationsHandler.php b/inc/Smartling/Services/ContentRelationsHandler.php index f67928f5f..8e5b0c4b4 100644 --- a/inc/Smartling/Services/ContentRelationsHandler.php +++ b/inc/Smartling/Services/ContentRelationsHandler.php @@ -3,7 +3,6 @@ namespace Smartling\Services; use Exception; -use Smartling\Exception\SmartlingDbException; use Smartling\Exception\SmartlingHumanReadableException; use Smartling\Helpers\AjaxAuthorizationFailure; use Smartling\Helpers\AjaxSecurityChecker; @@ -102,8 +101,8 @@ public function createSubmissionsHandler(array $data = null): void try { $this->service->createSubmissions(UserTranslationRequest::fromArray($data)); $this->returnResponse(['status' => BaseAjaxServiceAbstract::RESPONSE_SUCCESS]); - } catch (SmartlingDbException $e) { - $this->returnError('content.submission.failed', 'Invalid translation profile'); + } catch (SmartlingHumanReadableException $e) { + $this->returnError($e->getKey(), $e->getMessage(), $e->getResponseCode()); } catch (Exception $e) { $this->returnError('content.submission.failed', $e->getMessage()); } diff --git a/inc/Smartling/Settings/SettingsManager.php b/inc/Smartling/Settings/SettingsManager.php index 9b70eca5b..02a2d1a7f 100644 --- a/inc/Smartling/Settings/SettingsManager.php +++ b/inc/Smartling/Settings/SettingsManager.php @@ -10,6 +10,7 @@ use Smartling\Exception\EntityNotFoundException; use Smartling\Exception\SmartlingConfigException; use Smartling\Exception\SmartlingDbException; +use Smartling\Exception\SmartlingHumanReadableException; use Smartling\Helpers\ArrayHelper; use Smartling\Helpers\QueryBuilder\Condition\Condition; use Smartling\Helpers\QueryBuilder\Condition\ConditionBlock; @@ -124,30 +125,63 @@ public function getSingleSettingsProfile(int $mainBlogId): ConfigurationProfileE * An explicitly requested profile must exist, belong to the blog and be active, otherwise the request is rejected. * Without an explicit profile, the blog's active profile is used. * - * @throws SmartlingDbException + * Failures are reported as SmartlingHumanReadableException (rather than the broader SmartlingDbException thrown + * by unrelated DB/content errors elsewhere in the request) so callers can map profile-resolution failures to a + * clean 400 without mislabeling other failures as invalid profile errors. + * + * @throws SmartlingHumanReadableException */ public function resolveRequestedProfile(?int $requestedProfileId, int $blogId): ConfigurationProfileEntity { if ($requestedProfileId === null) { - return $this->getSingleSettingsProfile($blogId); + try { + return $this->getSingleSettingsProfile($blogId); + } catch (SmartlingDbException $e) { + throw new SmartlingHumanReadableException($e->getMessage(), 'profile.not.found', 400); + } } $profile = ArrayHelper::first($this->getEntityById($requestedProfileId)); if (!$profile instanceof ConfigurationProfileEntity) { $message = "Requested profileId=$requestedProfileId not found"; $this->getLogger()->warning($message); - throw new SmartlingDbException($message); + throw new SmartlingHumanReadableException($message, 'profile.invalid', 400); } if ($profile->getSourceLocale()->getBlogId() !== $blogId || 1 !== $profile->getIsActive()) { $message = "Requested profileId=$requestedProfileId is not an active profile for blogId=$blogId"; $this->getLogger()->warning($message); - throw new SmartlingDbException($message); + throw new SmartlingHumanReadableException($message, 'profile.invalid', 400); } $this->getLogger()->debug("Using requested profileId=$requestedProfileId for blogId=$blogId"); return $profile; } + /** + * @param int[] $targetBlogIds + * @throws SmartlingHumanReadableException + */ + public function assertTargetBlogIdsBelongToProfile(ConfigurationProfileEntity $profile, array $targetBlogIds): void + { + $allowedBlogIds = []; + foreach ($profile->getTargetLocales() as $locale) { + if ($locale->isEnabled()) { + $allowedBlogIds[] = $locale->getBlogId(); + } + } + + $invalidBlogIds = array_diff($targetBlogIds, $allowedBlogIds); + if (0 < count($invalidBlogIds)) { + $message = sprintf( + 'Target blogId(s) %s are not enabled target locales for profileId=%d', + implode(',', $invalidBlogIds), + $profile->getId(), + ); + $this->getLogger()->warning($message); + throw new SmartlingHumanReadableException($message, 'target.blog.invalid', 400); + } + } + /** * 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. diff --git a/inc/Smartling/Submissions/SubmissionManager.php b/inc/Smartling/Submissions/SubmissionManager.php index 426aa6b77..3ff341159 100644 --- a/inc/Smartling/Submissions/SubmissionManager.php +++ b/inc/Smartling/Submissions/SubmissionManager.php @@ -450,6 +450,12 @@ public function createSubmission(array $fields): SubmissionEntity /** * Loads from database or creates a new instance of SubmissionEntity + * + * When $profileId is given (e.g. the profile the parent submission was requested under, for related content + * created while processing it), it is stamped unconditionally, since the caller knows better than any default. + * Otherwise, an entity that already has a stamped profile is left alone - only a submission with none gets the + * source blog's active profile as a best-effort default. Re-stamping an already-stamped submission would make + * it diverge from the batch/job it was actually created under. */ public function getSubmissionEntity( string $contentType, @@ -457,7 +463,8 @@ public function getSubmissionEntity( int $sourceEntity, int $targetBlog, ?LocalizationPluginProxyInterface $localizationProxy = null, - ?int $targetEntity = null + ?int $targetEntity = null, + ?int $profileId = null, ): SubmissionEntity { $params = [ @@ -486,7 +493,12 @@ public function getSubmissionEntity( $entity->setSourceTitle('no title'); $entity->setCreatedAt(DateTimeHelper::nowAsString()); } - $this->stampConfigurationProfile($entity); + + if ($profileId !== null) { + $entity->setConfigurationProfileId($profileId); + } elseif ($entity->getConfigurationProfileId() === null) { + $this->stampConfigurationProfile($entity); + } return $entity; } diff --git a/inc/Smartling/WP/Controller/ContentEditJobController.php b/inc/Smartling/WP/Controller/ContentEditJobController.php index 73c090032..c5a0c4998 100644 --- a/inc/Smartling/WP/Controller/ContentEditJobController.php +++ b/inc/Smartling/WP/Controller/ContentEditJobController.php @@ -7,7 +7,7 @@ use Smartling\ApiWrapperInterface; use Smartling\Bootstrap; use Smartling\DbAl\LocalizationPluginProxyInterface; -use Smartling\Exception\SmartlingDbException; +use Smartling\Exception\SmartlingHumanReadableException; use Smartling\Exceptions\SmartlingApiException; use Smartling\Helpers\ArrayHelper; use Smartling\Helpers\Cache; @@ -110,7 +110,7 @@ public function initJobApiProxy(): void UserTranslationRequest::parseProfileId($params['profileId'] ?? null), $blogId, ); - } catch (\InvalidArgumentException | SmartlingDbException $e) { + } catch (\InvalidArgumentException | SmartlingHumanReadableException $e) { $this->getLogger()->warning('Unable to resolve requested profile: ' . $e->getMessage()); $this->wpProxy->wp_send_json(['status' => 400, 'message' => ['profileId' => 'Invalid translation profile']], 400); return; @@ -156,10 +156,18 @@ public function initJobApiProxy(): void $timezone = $validateRequires('timezone'); $jobDescription = $params['description']; $jobDueDate = $params['dueDate']; - $jobLocalesRaw = explode(',', $validateRequires('locales')); + $jobLocalesRaw = array_map('intval', explode(',', $validateRequires('locales'))); + if ($result['status'] === 200) { + try { + $this->settingsManager->assertTargetBlogIdsBelongToProfile($profile, $jobLocalesRaw); + } catch (SmartlingHumanReadableException $e) { + $result['status'] = 400; + $result['message']['locales'] = $e->getMessage(); + } + } $jobLocales = []; foreach ($jobLocalesRaw as $blogId) { - $jobLocales[] = $this->settingsManager->getSmartlingLocaleIdBySettingsProfile($profile, (int)$blogId); + $jobLocales[] = $this->settingsManager->getSmartlingLocaleIdBySettingsProfile($profile, $blogId); } if ($result['status'] === 200) { try { diff --git a/inc/Smartling/WP/Controller/InstantTranslationController.php b/inc/Smartling/WP/Controller/InstantTranslationController.php index 69e3c2523..b36ed722c 100644 --- a/inc/Smartling/WP/Controller/InstantTranslationController.php +++ b/inc/Smartling/WP/Controller/InstantTranslationController.php @@ -2,7 +2,7 @@ namespace Smartling\WP\Controller; -use Smartling\Exception\SmartlingDbException; +use Smartling\Exception\SmartlingHumanReadableException; use Smartling\FTS\FtsService; use Smartling\Helpers\AjaxSecurityChecker; use Smartling\Helpers\DateTimeHelper; @@ -73,15 +73,17 @@ public function handleRequestTranslation(): void $sourceBlogId = $this->wpProxy->get_current_blog_id(); try { - $profileId = $this->settingsManager->resolveRequestedProfile( + $profile = $this->settingsManager->resolveRequestedProfile( UserTranslationRequest::parseProfileId($_POST['profileId'] ?? null), $sourceBlogId, - )->getId(); - } catch (\InvalidArgumentException | SmartlingDbException $e) { + ); + $this->settingsManager->assertTargetBlogIdsBelongToProfile($profile, $targetBlogIds); + } catch (\InvalidArgumentException | SmartlingHumanReadableException $e) { $this->getLogger()->warning('Unable to resolve requested profile: ' . $e->getMessage()); $this->wpProxy->wp_send_json_error(['message' => 'Invalid translation profile'], 400); return; } + $profileId = $profile->getId(); $allSubmissions = $this->buildSubmissions( $contentType, @@ -317,6 +319,17 @@ private function getOrCreateSubmission( $submission = $this->submissionFactory->fromArray($submissionArray); $submission->setFileUri($this->fileUriHelper->generateFileUri($submission)); } else { + $oldProfileId = $submission->getConfigurationProfileId(); + if ($oldProfileId !== null && $oldProfileId !== $profileId + && $submission->getStatus() === SubmissionEntity::SUBMISSION_STATUS_IN_PROGRESS + ) { + $this->getLogger()->warning(sprintf( + 'Resubmitting submissionId=%d from profileId=%d to profileId=%d while still in progress; it will lose its link to the previous profile\'s file/job.', + $submission->getId(), + $oldProfileId, + $profileId, + )); + } $submission->setStatus(SubmissionEntity::SUBMISSION_STATUS_NEW); } diff --git a/inc/Smartling/WP/Controller/PostBasedWidgetControllerStd.php b/inc/Smartling/WP/Controller/PostBasedWidgetControllerStd.php index f90f24f8c..1cf7ffed7 100644 --- a/inc/Smartling/WP/Controller/PostBasedWidgetControllerStd.php +++ b/inc/Smartling/WP/Controller/PostBasedWidgetControllerStd.php @@ -6,6 +6,7 @@ use Smartling\Base\SmartlingCore; use Smartling\Bootstrap; use Smartling\Exception\SmartlingDbException; +use Smartling\Exception\SmartlingHumanReadableException; use Smartling\Extensions\Acf\AcfDynamicSupport; use Smartling\Helpers\ArrayHelper; use Smartling\Helpers\CommonLogMessagesTrait; @@ -280,6 +281,39 @@ public function ajaxUploadHandler() } } + /** + * Picking the profile that actually covers every selected target blog - more than one profile can be + * active for this site, and the earlier $profile (the first active one) may not be it. + */ + if ($continue) { + $targetBlogIds = array_map('intval', $data['blogs']); + $matchedProfile = null; + foreach ($this->getProfiles() as $candidateProfile) { + try { + $this->settingsManager->assertTargetBlogIdsBelongToProfile($candidateProfile, $targetBlogIds); + $matchedProfile = $candidateProfile; + break; + } catch (SmartlingHumanReadableException) { + continue; + } + } + + if ($matchedProfile === null) { + $message = 'Selected target locales are not all covered by a single translation profile.'; + $this->getLogger()->error( + vsprintf('Failed adding content to upload queue: %s for %s', [$message, var_export($_POST, true)]) + ); + $result = [ + 'status' => 'FAIL', + 'key' => self::ERROR_KEY_TARGET_BLOG_EMPTY, + 'message' => $message, + ]; + $continue = false; + } else { + $profile = $matchedProfile; + } + } + if ($continue) { $data['content']['id'] = explode(',', $data['content']['id']); @@ -355,9 +389,9 @@ public function ajaxUploadHandler() try { $jobInfo = new JobEntityWithBatchUid('', $jobName, $data['job']['id'], $profile->getProjectId()); if ($this->getCore()->getTranslationHelper()->isRelatedSubmissionCreationNeeded($contentType, $sourceBlog, (int)$sourceId, (int)$targetBlogId)) { - $submission = $this->getCore()->getTranslationHelper()->tryPrepareRelatedContent($contentType, $sourceBlog, (int)$sourceId, (int)$targetBlogId, $jobInfo); + $submission = $this->getCore()->getTranslationHelper()->tryPrepareRelatedContent($contentType, $sourceBlog, (int)$sourceId, (int)$targetBlogId, $jobInfo, false, $profile->getId()); } else { - $submission = $this->getCore()->getTranslationHelper()->getExistingSubmissionOrCreateNew($contentType, $sourceBlog, (int)$sourceId, (int)$targetBlogId, $jobInfo); + $submission = $this->getCore()->getTranslationHelper()->getExistingSubmissionOrCreateNew($contentType, $sourceBlog, (int)$sourceId, (int)$targetBlogId, $jobInfo, $profile->getId()); } if (0 < $submission->getId()) { @@ -434,6 +468,12 @@ public function ajaxRefreshWidgetHandler(): void wp_send_json(['status' => self::RESPONSE_AJAX_STATUS_FAIL, 'message' => 'Post not found'], 404); return; } + if ($post->post_type !== $this->servedContentType) { + // One instance of this controller is registered per post type, all on the same AJAX + // action. Responding here would short-circuit the request before the instance whose + // servedContentType actually matches this post gets a turn, so just let it fall through. + return; + } if (!current_user_can('edit_post', $post->ID)) { $this->getLogger()->warning(sprintf('User %d cannot edit post %d', get_current_user_id(), $post->ID)); wp_send_json(['status' => self::RESPONSE_AJAX_STATUS_FAIL, 'message' => 'Insufficient permissions'], 403); diff --git a/inc/Smartling/WP/View/ContentEditJob.php b/inc/Smartling/WP/View/ContentEditJob.php index 9127d2661..f3726f9eb 100644 --- a/inc/Smartling/WP/View/ContentEditJob.php +++ b/inc/Smartling/WP/View/ContentEditJob.php @@ -46,6 +46,7 @@
No suitable target locales found.
Please check your settings. + href="/wp-admin/admin.php?page=">settings.
diff --git a/tests/Services/ContentRelationsHandlerTest.php b/tests/Services/ContentRelationsHandlerTest.php index 4ace2bdac..f9c5d80bf 100644 --- a/tests/Services/ContentRelationsHandlerTest.php +++ b/tests/Services/ContentRelationsHandlerTest.php @@ -3,6 +3,8 @@ namespace Smartling\Tests\Services; use PHPUnit\Framework\TestCase; +use Smartling\Exception\SmartlingDbException; +use Smartling\Exception\SmartlingHumanReadableException; use Smartling\Helpers\AjaxSecurityChecker; use Smartling\Helpers\ArrayHelper; use Smartling\Helpers\WordpressFunctionProxyHelper; @@ -103,6 +105,75 @@ public function returnError($key, $message, $responseCode = 400): void $this->assertSame(403, $x->capturedErrorCode); } + /** + * A SmartlingHumanReadableException thrown while resolving the profile or validating target blogs must surface + * its own key/message/response code to the client, not be swallowed into a generic failure. + */ + public function testCreateSubmissionsHandlerMapsHumanReadableExceptionToItsOwnKeyAndCode(): void + { + $service = $this->createMock(ContentRelationsDiscoveryService::class); + $service->method('createSubmissions')->willThrowException( + new SmartlingHumanReadableException('Invalid target locale for selected profile', 'target.blog.invalid', 400) + ); + $proxy = $this->makeWpProxy(); + + $x = new class($service, $proxy, new AjaxSecurityChecker($proxy)) extends ContentRelationsHandler { + public ?string $capturedErrorKey = null; + public ?string $capturedErrorMessage = null; + public ?int $capturedErrorCode = null; + + public function returnResponse(array $data, $responseCode = 200): void + { + TestCase::fail('Should not return a success response'); + } + + public function returnError($key, $message, $responseCode = 400): void + { + $this->capturedErrorKey = $key; + $this->capturedErrorMessage = $message; + $this->capturedErrorCode = $responseCode; + } + }; + + $x->createSubmissionsHandler($this->buildData(['source' => ['id' => [1], 'contentType' => 'post'], 'targetBlogIds' => '2'])); + + $this->assertSame('target.blog.invalid', $x->capturedErrorKey); + $this->assertSame('Invalid target locale for selected profile', $x->capturedErrorMessage); + $this->assertSame(400, $x->capturedErrorCode); + } + + /** + * A SmartlingDbException from unrelated code (DB layer, queue, content handlers, etc.) reachable from + * createSubmissions() must not be mislabeled as a profile error - its real message must still reach the client. + */ + public function testCreateSubmissionsHandlerPreservesMessageForUnrelatedDbException(): void + { + $service = $this->createMock(ContentRelationsDiscoveryService::class); + $service->method('createSubmissions')->willThrowException(new SmartlingDbException('Queue table is locked')); + $proxy = $this->makeWpProxy(); + + $x = new class($service, $proxy, new AjaxSecurityChecker($proxy)) extends ContentRelationsHandler { + public ?string $capturedErrorKey = null; + public ?string $capturedErrorMessage = null; + + public function returnResponse(array $data, $responseCode = 200): void + { + TestCase::fail('Should not return a success response'); + } + + public function returnError($key, $message, $responseCode = 400): void + { + $this->capturedErrorKey = $key; + $this->capturedErrorMessage = $message; + } + }; + + $x->createSubmissionsHandler($this->buildData(['source' => ['id' => [1], 'contentType' => 'post'], 'targetBlogIds' => '2'])); + + $this->assertSame('content.submission.failed', $x->capturedErrorKey); + $this->assertSame('Queue table is locked', $x->capturedErrorMessage); + } + private function buildData(array $overrides = []): array { return array_merge([ diff --git a/tests/Smartling/Helpers/MetaFieldProcessor/ReferencedContentProcessorTest.php b/tests/Smartling/Helpers/MetaFieldProcessor/ReferencedContentProcessorTest.php new file mode 100644 index 000000000..eea8cbe98 --- /dev/null +++ b/tests/Smartling/Helpers/MetaFieldProcessor/ReferencedContentProcessorTest.php @@ -0,0 +1,90 @@ +createMock(ContentHelper::class), + $this->getMockBuilder(SubmissionManager::class)->disableOriginalConstructor()->getMock(), + $translationHelper, + '/.*/', + 'post', + ); + } + + private function parentSubmission(?int $profileId): SubmissionEntity + { + $submission = (new SubmissionEntity()) + ->setSourceBlogId(1) + ->setTargetBlogId(2) + ->setConfigurationProfileId($profileId); + $submission->setJobInfo(new JobEntity('Test job', 'jobUid', 'projectUid')); + + return $submission; + } + + /** + * Related content created while processing a submission must be stamped with the parent submission's own + * profile, not whatever the source blog's default happens to be - otherwise related content silently ends up + * under a different Smartling project than the one the user actually requested. + */ + public function testProcessFieldPreTranslationPassesParentSubmissionProfileIdToRelatedContent(): void + { + $parent = $this->parentSubmission(7); + + $translationHelper = $this->createMock(TranslationHelper::class); + $translationHelper->method('isRelatedSubmissionCreationNeeded')->willReturn(true); + $translationHelper->expects($this->once()) + ->method('tryPrepareRelatedContent') + ->with( + 'post', + 1, + 42, + 2, + $this->isInstanceOf(JobEntityWithBatchUid::class), + false, + 7, + ) + ->willReturn($parent); + + $processor = $this->getProcessor($translationHelper); + + $processor->processFieldPreTranslation($parent, 'some_field', 42, []); + } + + public function testProcessFieldPreTranslationPassesNullProfileIdWhenParentHasNone(): void + { + $parent = $this->parentSubmission(null); + + $translationHelper = $this->createMock(TranslationHelper::class); + $translationHelper->method('isRelatedSubmissionCreationNeeded')->willReturn(true); + $translationHelper->expects($this->once()) + ->method('tryPrepareRelatedContent') + ->with( + 'post', + 1, + 42, + 2, + $this->isInstanceOf(JobEntityWithBatchUid::class), + false, + null, + ) + ->willReturn($parent); + + $processor = $this->getProcessor($translationHelper); + + $processor->processFieldPreTranslation($parent, 'some_field', 42, []); + } +} diff --git a/tests/Smartling/Settings/SettingsManagerTest.php b/tests/Smartling/Settings/SettingsManagerTest.php index 330b797a9..511e5dd46 100644 --- a/tests/Smartling/Settings/SettingsManagerTest.php +++ b/tests/Smartling/Settings/SettingsManagerTest.php @@ -7,6 +7,7 @@ use Smartling\DbAl\SmartlingToCMSDatabaseAccessWrapperInterface; use Smartling\Exception\SmartlingConfigException; use Smartling\Exception\SmartlingDbException; +use Smartling\Exception\SmartlingHumanReadableException; use Smartling\Settings\ConfigurationProfileEntity; use Smartling\Settings\Locale; use Smartling\Settings\SettingsManager; @@ -209,7 +210,7 @@ public function testResolveRequestedProfileUsesRequestedWhenValidAndActive() public function testResolveRequestedProfileRejectsUnknownProfile() { - $this->expectException(SmartlingDbException::class); + $this->expectException(SmartlingHumanReadableException::class); $mock = $this->resolverMock(); $mock->method('getEntityById')->with(5)->willReturn([]); @@ -218,7 +219,7 @@ public function testResolveRequestedProfileRejectsUnknownProfile() public function testResolveRequestedProfileRejectsProfileOfDifferentBlog() { - $this->expectException(SmartlingDbException::class); + $this->expectException(SmartlingHumanReadableException::class); $mock = $this->resolverMock(); $mock->method('getEntityById')->with(5)->willReturn([$this->profileForBlog(5, 99, 1)]); @@ -227,7 +228,7 @@ public function testResolveRequestedProfileRejectsProfileOfDifferentBlog() public function testResolveRequestedProfileRejectsInactiveProfile() { - $this->expectException(SmartlingDbException::class); + $this->expectException(SmartlingHumanReadableException::class); $mock = $this->resolverMock(); $mock->method('getEntityById')->with(5)->willReturn([$this->profileForBlog(5, 1, 0)]); @@ -246,13 +247,54 @@ public function testResolveRequestedProfileFallsBackToActiveProfileWhenNoneReque public function testResolveRequestedProfileThrowsWhenNoneRequestedAndNoneActive() { - $this->expectException(SmartlingDbException::class); + $this->expectException(SmartlingHumanReadableException::class); $mock = $this->resolverMock(); $mock->method('findEntityByMainLocale')->with(1)->willReturn([]); $mock->resolveRequestedProfile(null, 1); } + public function testAssertTargetBlogIdsBelongToProfileAllowsEnabledLocales() + { + $profile = $this->profileWithId(1); + $profile->setTargetLocales([$this->enabledLocale(2), $this->enabledLocale(3)]); + $mock = $this->resolverMock(); + + $mock->assertTargetBlogIdsBelongToProfile($profile, [2, 3]); + $this->addToAssertionCount(1); + } + + public function testAssertTargetBlogIdsBelongToProfileRejectsBlogOutsideProfile() + { + $this->expectException(SmartlingHumanReadableException::class); + $profile = $this->profileWithId(1); + $profile->setTargetLocales([$this->enabledLocale(2)]); + $mock = $this->resolverMock(); + + $mock->assertTargetBlogIdsBelongToProfile($profile, [2, 99]); + } + + public function testAssertTargetBlogIdsBelongToProfileRejectsDisabledLocale() + { + $this->expectException(SmartlingHumanReadableException::class); + $profile = $this->profileWithId(1); + $disabled = $this->enabledLocale(2); + $disabled->setEnabled(false); + $profile->setTargetLocales([$disabled]); + $mock = $this->resolverMock(); + + $mock->assertTargetBlogIdsBelongToProfile($profile, [2]); + } + + private function enabledLocale(int $blogId): TargetLocale + { + $locale = new TargetLocale(); + $locale->setBlogId($blogId); + $locale->setEnabled(true); + + return $locale; + } + public function testGetEntitiesQueries() { $db = $this->createMock(SmartlingToCMSDatabaseAccessWrapperInterface::class); diff --git a/tests/Smartling/Submissions/SubmissionManagerTest.php b/tests/Smartling/Submissions/SubmissionManagerTest.php index 325b9849c..c753dbe65 100644 --- a/tests/Smartling/Submissions/SubmissionManagerTest.php +++ b/tests/Smartling/Submissions/SubmissionManagerTest.php @@ -171,14 +171,25 @@ public function testGetSubmissionEntityStoresProfileOnNewSubmission() $this->assertSame(9, $entity->getConfigurationProfileId()); } - public function testGetSubmissionEntityRefreshesProfileOnExistingSubmission() + public function testGetSubmissionEntityKeepsProfileOnExistingSubmission() { $settingsManager = $this->createMock(SettingsManager::class); - $settingsManager->method('getSingleSettingsProfile')->willReturn($this->profileWithId(9)); + $settingsManager->expects($this->never())->method('getSingleSettingsProfile'); $existing = (new SubmissionEntity())->setSourceBlogId(1)->setConfigurationProfileId(4); $entity = $this->getManagerForProfileStamping($settingsManager, [$existing])->getSubmissionEntity('post', 1, 5, 2); + $this->assertSame(4, $entity->getConfigurationProfileId()); + } + + public function testGetSubmissionEntityStampsExplicitProfileIdEvenOnExistingSubmission() + { + $settingsManager = $this->createMock(SettingsManager::class); + $settingsManager->expects($this->never())->method('getSingleSettingsProfile'); + $existing = (new SubmissionEntity())->setSourceBlogId(1)->setConfigurationProfileId(4); + + $entity = $this->getManagerForProfileStamping($settingsManager, [$existing])->getSubmissionEntity('post', 1, 5, 2, null, null, 9); + $this->assertSame(9, $entity->getConfigurationProfileId()); } From ab8892e7e9df22de3a0a24f13a3b32cb94fdec26 Mon Sep 17 00:00:00 2001 From: Vitalii Solovei Date: Thu, 8 Oct 2026 19:13:50 +0200 Subject: [PATCH 12/14] address review: widget refresh and profile-switch UI fixes (WP-1022) - Swapping in the refreshed widget via outerHTML replaced the DOM node smartling-connector-admin.js had bound its Download click handler to, silently breaking the button after the first refresh. Delegate that binding from document instead, so it survives the node being replaced. - The per-post-type widget controller that registers smartling_refresh_post_widget is instantiated once per post type, so every instance was handling every request and rendering its own content type's (usually empty) submissions for any other type's post. Each instance now returns early when the requested post isn't its own served content type, letting the matching one respond. - The job wizard also renders on taxonomy term edit screens, where !isBulkSubmitPage is true but there is no post behind the term id - the refresh call was sending a term id as postId. Gate it on a new data-base-type attribute so it only fires for actual posts. - On a profile switch, the previous profile's jobs stayed selectable (and a prior load error stayed visible) until the new list arrived, or forever on failure. Clear both up front instead of waiting for the response. Co-Authored-By: Claude Sonnet 5 --- js/app.js | 14 ++++++++++---- js/smartling-connector-admin.js | 5 ++++- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/js/app.js b/js/app.js index 94c3344a1..3e59cbaf7 100644 --- a/js/app.js +++ b/js/app.js @@ -61,7 +61,7 @@ async function refreshDownloadWidgetUntilReady(adminUrl, postId, targetBlogIds) } } -function JobWizard({ isBulkSubmitPage, contentType, contentId, profiles, blogId, ajaxUrl, adminUrl, nonce }) { +function JobWizard({ isBulkSubmitPage, baseType, contentType, contentId, profiles, blogId, ajaxUrl, adminUrl, nonce }) { const [activeTab, setActiveTab] = useState('new'); const [selectedProfileId, setSelectedProfileId] = useState(() => { const stored = getStoredProfileId(blogId); @@ -93,8 +93,13 @@ function JobWizard({ isBulkSubmitPage, contentType, contentId, profiles, blogId, const [instantCompletedCount, setInstantCompletedCount] = useState(0); useEffect(() => { - // Ignore responses for a previously selected profile, a slow one must not overwrite the current job list + // Ignore responses for a previously selected profile, a slow one must not overwrite the current job list. + // The old profile's jobs must not stay selectable while the new list loads (or forever, if it fails), so + // clear them - and any stale error from a previous attempt - up front rather than waiting for the response. let stale = false; + setJobs([]); + setLoading(true); + setError(''); (async () => { try { const response = await jQuery.post(adminUrl, { @@ -396,7 +401,7 @@ function JobWizard({ isBulkSubmitPage, contentType, contentId, profiles, blogId, throw new Error(submissionResponse.message?.global || 'Failed to add content to upload queue.'); } setSuccess('Content successfully added to upload queue.'); - if (!isBulkSubmitPage) { + if (!isBulkSubmitPage && baseType === 'post') { refreshDownloadWidgetUntilReady(adminUrl, contentId, selectedLocales); } } catch (e) { @@ -603,6 +608,7 @@ function JobWizard({ isBulkSubmitPage, contentType, contentId, profiles, blogId, if (document.getElementById('smartling-app')) { const container = document.getElementById('smartling-app'); const isBulkSubmitPage = container.dataset.bulkSubmit === 'true'; + const baseType = container.dataset.baseType || 'post'; const contentType = container.dataset.contentType || ''; const contentId = parseInt(container.dataset.contentId, 10) || 0; const profiles = JSON.parse(container.dataset.profiles || '[]'); @@ -614,7 +620,7 @@ if (document.getElementById('smartling-app')) { // Nothing to offer without an active profile if (profiles.length > 0) { render( - el(JobWizard, { isBulkSubmitPage, contentType, contentId, profiles, blogId, ajaxUrl, adminUrl, nonce }), + el(JobWizard, { isBulkSubmitPage, baseType, contentType, contentId, profiles, blogId, ajaxUrl, adminUrl, nonce }), container ); } diff --git a/js/smartling-connector-admin.js b/js/smartling-connector-admin.js index 02ec90ce9..72d19a419 100644 --- a/js/smartling-connector-admin.js +++ b/js/smartling-connector-admin.js @@ -193,7 +193,10 @@ var downloadSelector = "#smartling-download"; localizationOptions.init(); } if ($(localizationOptions.selectors.post_widget).length > 0) { - $(localizationOptions.selectors.download).on("click", function () { + // Delegated from document (rather than bound directly to the button) so the handler + // survives the widget being replaced wholesale after a refresh (see refreshDownloadWidgetUntilReady + // in app.js), which swaps in a brand new, unbound #smartling-download element. + $(document).on("click", localizationOptions.selectors.download, function () { ajaxDownload(); }); } From 50715fc03dfa6319a31567ed70ca86dbb1d73016 Mon Sep 17 00:00:00 2001 From: Vitalii Solovei Date: Fri, 9 Oct 2026 12:53:48 +0200 Subject: [PATCH 13/14] address review: drop widget auto-refresh, fix profile inference and relation staleness (WP-1022) The post-upload widget auto-refresh (smartling_refresh_post_widget, refreshDownloadWidgetUntilReady) caused more problems than it solved - duplicate nonce/referer inputs accumulating in the post edit form on each poll (overriding the real referer on submit), a handler-binding footgun from swapping the widget via outerHTML, and the multi-post-type AJAX registration collision. Removed it entirely rather than patching further: the ajaxRefreshWidgetHandler endpoint, its frontend poller, the data-base-type plumbing that only existed to gate it, and the Download button's delegated-from-document binding that only existed to survive the outerHTML swap. Also, from the latest review round: - post-based-content-type.php's multi-profile locale merge used `??=` before checking isEnabled(), so a disabled entry from the first profile could shadow an enabled one from a later profile. Now only enabled locales are merged. - The legacy widget's ajaxUploadHandler() inferred the profile by searching for one that happens to cover the selected blogs, which ignores which project the job actually belongs to. It now resolves via resolveRequestedProfile() (accepting an explicit profileId) and validates target blogs with assertTargetBlogIdsBelongToProfile(), consistent with the other three entry points. - Switching profiles in the job wizard left relations fetched for the old profile's target locales in place, so the new fetch appended instead of replacing and the progress bar's totalRequests kept growing. handleProfileChange now resets l1Relations, l2Relations, selectedRelations, pendingRequests and totalRequests. Co-Authored-By: Claude Sonnet 5 --- .../PostBasedWidgetControllerStd.php | 107 +++++------------- inc/Smartling/WP/View/ContentEditJob.php | 1 - .../WP/View/post-based-content-type.php | 4 +- js/app.js | 57 ++-------- js/smartling-connector-admin.js | 5 +- 5 files changed, 44 insertions(+), 130 deletions(-) diff --git a/inc/Smartling/WP/Controller/PostBasedWidgetControllerStd.php b/inc/Smartling/WP/Controller/PostBasedWidgetControllerStd.php index 1cf7ffed7..fe55c8fb0 100644 --- a/inc/Smartling/WP/Controller/PostBasedWidgetControllerStd.php +++ b/inc/Smartling/WP/Controller/PostBasedWidgetControllerStd.php @@ -14,6 +14,7 @@ use Smartling\Helpers\DiagnosticsHelper; use Smartling\Helpers\SmartlingUserCapabilities; use Smartling\Jobs\JobEntityWithBatchUid; +use Smartling\Models\UserTranslationRequest; use Smartling\Submissions\SubmissionEntity; use Smartling\Vendor\Smartling\AuditLog\Params\CreateRecordParameters; use Smartling\WP\WPAbstract; @@ -218,25 +219,33 @@ public function ajaxUploadHandler() } } - $profile = ArrayHelper::first($this->getProfiles()); + $profile = null; /** - * checking profiles + * checking profile - resolves the explicit profileId when the caller sends one (consistent with every + * other entry point), falling back to the blog's single active profile otherwise. */ - if ($continue && !$profile) { - $this->getLogger()->error( - vsprintf( - 'Failed adding content to upload queue: %s for %s', - [self::ERROR_MSG_NO_PROFILE_FOUND, var_export($_POST, true)]) - ); + if ($continue) { + try { + $profile = $this->settingsManager->resolveRequestedProfile( + UserTranslationRequest::parseProfileId($data['profileId'] ?? null), + $this->siteHelper->getCurrentBlogId(), + ); + } catch (\InvalidArgumentException | SmartlingHumanReadableException $e) { + $this->getLogger()->error( + vsprintf( + 'Failed adding content to upload queue: %s for %s', + [$e->getMessage(), var_export($_POST, true)]) + ); - $result = [ - 'status' => 'FAIL', - 'key' => self::ERROR_KEY_NO_PROFILE_FOUND, - 'message' => self::ERROR_MSG_NO_PROFILE_FOUND, - ]; + $result = [ + 'status' => 'FAIL', + 'key' => self::ERROR_KEY_NO_PROFILE_FOUND, + 'message' => self::ERROR_MSG_NO_PROFILE_FOUND, + ]; - $continue = false; + $continue = false; + } } /** @@ -282,35 +291,22 @@ public function ajaxUploadHandler() } /** - * Picking the profile that actually covers every selected target blog - more than one profile can be - * active for this site, and the earlier $profile (the first active one) may not be it. + * Validates that every selected target blog is actually covered by the resolved profile - the profile is + * either the one explicitly requested or the blog's single active one, never inferred from the blogs. */ if ($continue) { - $targetBlogIds = array_map('intval', $data['blogs']); - $matchedProfile = null; - foreach ($this->getProfiles() as $candidateProfile) { - try { - $this->settingsManager->assertTargetBlogIdsBelongToProfile($candidateProfile, $targetBlogIds); - $matchedProfile = $candidateProfile; - break; - } catch (SmartlingHumanReadableException) { - continue; - } - } - - if ($matchedProfile === null) { - $message = 'Selected target locales are not all covered by a single translation profile.'; + try { + $this->settingsManager->assertTargetBlogIdsBelongToProfile($profile, array_map('intval', $data['blogs'])); + } catch (SmartlingHumanReadableException $e) { $this->getLogger()->error( - vsprintf('Failed adding content to upload queue: %s for %s', [$message, var_export($_POST, true)]) + vsprintf('Failed adding content to upload queue: %s for %s', [$e->getMessage(), var_export($_POST, true)]) ); $result = [ 'status' => 'FAIL', 'key' => self::ERROR_KEY_TARGET_BLOG_EMPTY, - 'message' => $message, + 'message' => $e->getMessage(), ]; $continue = false; - } else { - $profile = $matchedProfile; } } @@ -441,52 +437,9 @@ public function register(): void add_action('save_post', [$this, 'save']); // old logic 2 be refactored add_action('wp_ajax_' . 'smartling_force_download_handler', [$this, 'ajaxDownloadHandler']); add_action('wp_ajax_' . 'smartling_upload_handler', [$this, 'ajaxUploadHandler']); - add_action('wp_ajax_' . 'smartling_refresh_post_widget', [$this, 'ajaxRefreshWidgetHandler']); } } - /** - * Re-renders the widget for a post so the caller can swap it into the DOM, picking up - * submissions/target placeholders created after the page was first loaded (e.g. by the - * upload job wizard) without a full page reload. - */ - public function ajaxRefreshWidgetHandler(): void - { - if (check_ajax_referer(self::AJAX_NONCE_ACTION, '_wpnonce', false) === false) { - $this->getLogger()->warning(sprintf('Invalid nonce for action "%s" from userId=%d', self::AJAX_NONCE_ACTION, get_current_user_id())); - wp_send_json(['status' => self::RESPONSE_AJAX_STATUS_FAIL, 'message' => 'Invalid nonce'], 403); - return; - } - if (!current_user_can(SmartlingUserCapabilities::SMARTLING_CAPABILITY_WIDGET_CAP)) { - $this->getLogger()->warning(sprintf('User %d lacks capability "%s"', get_current_user_id(), SmartlingUserCapabilities::SMARTLING_CAPABILITY_WIDGET_CAP)); - wp_send_json(['status' => self::RESPONSE_AJAX_STATUS_FAIL, 'message' => 'Insufficient permissions'], 403); - return; - } - - $post = get_post((int)($_POST['postId'] ?? 0)); - if (!($post instanceof \WP_Post)) { - wp_send_json(['status' => self::RESPONSE_AJAX_STATUS_FAIL, 'message' => 'Post not found'], 404); - return; - } - if ($post->post_type !== $this->servedContentType) { - // One instance of this controller is registered per post type, all on the same AJAX - // action. Responding here would short-circuit the request before the instance whose - // servedContentType actually matches this post gets a turn, so just let it fall through. - return; - } - if (!current_user_can('edit_post', $post->ID)) { - $this->getLogger()->warning(sprintf('User %d cannot edit post %d', get_current_user_id(), $post->ID)); - wp_send_json(['status' => self::RESPONSE_AJAX_STATUS_FAIL, 'message' => 'Insufficient permissions'], 403); - return; - } - - ob_start(); - $this->preView($post); - $html = ob_get_clean(); - - wp_send_json(['status' => self::RESPONSE_AJAX_STATUS_SUCCESS, 'html' => $html]); - } - /** * @var SmartlingCore */ diff --git a/inc/Smartling/WP/View/ContentEditJob.php b/inc/Smartling/WP/View/ContentEditJob.php index f3726f9eb..9127d2661 100644 --- a/inc/Smartling/WP/View/ContentEditJob.php +++ b/inc/Smartling/WP/View/ContentEditJob.php @@ -46,7 +46,6 @@
getTargetLocales() as $locale) { - $locales[$locale->getBlogId()] ??= $locale; + if ($locale->isEnabled()) { + $locales[$locale->getBlogId()] ??= $locale; + } } } $locales = array_values($locales); diff --git a/js/app.js b/js/app.js index 3e59cbaf7..8e43e9d96 100644 --- a/js/app.js +++ b/js/app.js @@ -20,48 +20,7 @@ function setStoredProfileId(blogId, profileId) { } } -async function refreshDownloadWidgetUntilReady(adminUrl, postId, targetBlogIds) { - if (!document.getElementById('smartling-post-widget') || !targetBlogIds.length) { - return; - } - - const DELAYS = [3000, 5000, 10000, 15000, 15000]; - for (const delay of DELAYS) { - await new Promise(resolve => setTimeout(resolve, delay)); - - let response; - try { - response = await jQuery.post(adminUrl, { - action: 'smartling_refresh_post_widget', - postId, - _wpnonce: (typeof smartlingConnector !== 'undefined' ? smartlingConnector.nonce : '') - }); - } catch (e) { - continue; - } - if (response.status !== 'SUCCESS' || !response.html) { - continue; - } - - const widget = document.getElementById('smartling-post-widget'); - if (!widget) { - return; - } - widget.outerHTML = response.html; - - const updated = document.getElementById('smartling-post-widget'); - const allReady = targetBlogIds.every(blogId => { - const blogInput = updated?.querySelector(`input[name="smartling[locales][${blogId}][blog]"]`); - const row = blogInput?.closest('.smtPostWidget-rowWrapper'); - return row?.querySelector('.smtPostWidget-row a') != null; - }); - if (allReady) { - return; - } - } -} - -function JobWizard({ isBulkSubmitPage, baseType, contentType, contentId, profiles, blogId, ajaxUrl, adminUrl, nonce }) { +function JobWizard({ isBulkSubmitPage, contentType, contentId, profiles, blogId, ajaxUrl, adminUrl, nonce }) { const [activeTab, setActiveTab] = useState('new'); const [selectedProfileId, setSelectedProfileId] = useState(() => { const stored = getStoredProfileId(blogId); @@ -136,6 +95,14 @@ function JobWizard({ isBulkSubmitPage, baseType, contentType, contentId, profile setJobName(''); setDescription(''); setDueDate(''); + // Relations were fetched for the previous profile's target locales; clear them so the + // depth effect (keyed off the new locale list) starts a fresh fetch instead of appending + // to stale entries and inflating the progress bar's totalRequests count. + setL1Relations([]); + setL2Relations([]); + setSelectedRelations({}); + setPendingRequests(0); + setTotalRequests(0); }; const loadRelations = useCallback(async (type, id, level = 1) => { @@ -401,9 +368,6 @@ function JobWizard({ isBulkSubmitPage, baseType, contentType, contentId, profile throw new Error(submissionResponse.message?.global || 'Failed to add content to upload queue.'); } setSuccess('Content successfully added to upload queue.'); - if (!isBulkSubmitPage && baseType === 'post') { - refreshDownloadWidgetUntilReady(adminUrl, contentId, selectedLocales); - } } catch (e) { setError(e.message || 'Failed adding content to upload queue.'); } finally { @@ -608,7 +572,6 @@ function JobWizard({ isBulkSubmitPage, baseType, contentType, contentId, profile if (document.getElementById('smartling-app')) { const container = document.getElementById('smartling-app'); const isBulkSubmitPage = container.dataset.bulkSubmit === 'true'; - const baseType = container.dataset.baseType || 'post'; const contentType = container.dataset.contentType || ''; const contentId = parseInt(container.dataset.contentId, 10) || 0; const profiles = JSON.parse(container.dataset.profiles || '[]'); @@ -620,7 +583,7 @@ if (document.getElementById('smartling-app')) { // Nothing to offer without an active profile if (profiles.length > 0) { render( - el(JobWizard, { isBulkSubmitPage, baseType, contentType, contentId, profiles, blogId, ajaxUrl, adminUrl, nonce }), + el(JobWizard, { isBulkSubmitPage, contentType, contentId, profiles, blogId, ajaxUrl, adminUrl, nonce }), container ); } diff --git a/js/smartling-connector-admin.js b/js/smartling-connector-admin.js index 72d19a419..02ec90ce9 100644 --- a/js/smartling-connector-admin.js +++ b/js/smartling-connector-admin.js @@ -193,10 +193,7 @@ var downloadSelector = "#smartling-download"; localizationOptions.init(); } if ($(localizationOptions.selectors.post_widget).length > 0) { - // Delegated from document (rather than bound directly to the button) so the handler - // survives the widget being replaced wholesale after a refresh (see refreshDownloadWidgetUntilReady - // in app.js), which swaps in a brand new, unbound #smartling-download element. - $(document).on("click", localizationOptions.selectors.download, function () { + $(localizationOptions.selectors.download).on("click", function () { ajaxDownload(); }); } From 7c91702beb9ca184f60221a4b882df58248461ee Mon Sep 17 00:00:00 2001 From: Vitalii Solovei Date: Sat, 10 Oct 2026 11:22:41 +0200 Subject: [PATCH 14/14] add changelog entry (WP-1022) --- readme.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/readme.txt b/readme.txt index 297fba006..7e79c87d4 100755 --- a/readme.txt +++ b/readme.txt @@ -63,6 +63,7 @@ Additional information on the Smartling Connector for WordPress can be found [he == Changelog == = 5.8.1 = +* Added profile selector for sites with multiple active profiles. * Improved Elementor query support: term and post IDs used by Posts, Loop Grid and Loop Carousel queries are now detected and replaced with the translated ones, only for the query mode the widget actually uses. Query terms are stored by Elementor Pro as term taxonomy IDs and are converted accordingly. Excluded terms and posts are replaced only if they are already translated when the page translation is downloaded; if they are translated later, download the page again to update the query. * Added the `smartling_target_id` filter, which returns the translated ID of a post, attachment or term for use in custom code. * Fixed Elementor query terms being dropped from related content when the same taxonomy is also detected elsewhere in the document.