Skip to content

choose configuration profile UI (WP-1022) - #638

Open
vsolovei-smartling wants to merge 10 commits into
masterfrom
WP-1022-choose-profile-on-request
Open

vsolovei-smartling wants to merge 10 commits into
masterfrom
WP-1022-choose-profile-on-request

Conversation

@vsolovei-smartling

Copy link
Copy Markdown
Contributor

No description provided.

vsolovei-smartling and others added 9 commits October 6, 2026 11:49
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.
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.
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
…1022)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
… client (WP-1022)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

@sl-mmuradov sl-mmuradov left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, centralizing the lookup in resolveRequestedProfile() with a strict check (exists, same blog, active) is the right approach, and keeping error details server-side is good. A few things block merging, though:

Blocking

  1. Related content still uses the default profile. SubmissionManager::getSubmissionEntity() always stamps getSingleSettingsProfile(), even on existing submissions. Related content created during upload or download (referenced posts and terms, images, downloadTranslation) therefore goes to profile A while the parent and job are on B, and existing B submissions are re-stamped to A. This defeats the main goal of the ticket for any content with relations.
  2. Target blogs aren't validated against the chosen profile in createSubmissions, create-job or instant translation. The result is an empty Smartling locale and a failure later on.
  3. The widget refresh breaks the side widget. The outerHTML swap drops the Download and checkbox handlers. The AJAX action is registered by every post-type controller, so only the first post type renders correct submissions. Taxonomy screens poll with a term id as postId.
  4. ContentRelationsHandler reports every SmartlingDbException as "Invalid translation profile", which hides real DB errors.

Should fix

  • Re-stamping in-progress submissions under a different profile leaves them pointing at the wrong project. Block or warn.
  • The wizard leaves the old profile's jobs selectable after a switch, doesn't clear its error, and re-fires all relation requests.
  • The side widget merges locales from all profiles, but its legacy upload path uses only the first profile.

Scope question: the ticket asks for a per-blog default profile. Right now it's "first active by id" plus localStorage. Is that a follow-up ticket?

Tests: please add tests for ajaxRefreshWidgetHandler, the job proxy's profile 400 path, the handler's exception mapping, related-content stamping when the requested profile isn't the default, and Playwright coverage for the dropdown, the reset on switch and the widget refresh.

Inline comments have details and suggested fixes.

// 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.
// New submissions built from this template bypass getSubmissionEntity(), stamp them with the requested profile.
$submissionTemplateArray[SubmissionEntity::FIELD_CONFIGURATION_PROFILE_ID] = $profile->getId();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[High] Parent submissions are stamped with the requested profile here, but related content created later still gets the default profile. SubmissionManager::getSubmissionEntity() → stampConfigurationProfile() → getSingleSettingsProfile() (SubmissionManager.php ~489) runs for new and existing submissions. It's reached from TranslationHelper::tryPrepareRelatedContent() via ReferencedContentProcessor (referenced posts/terms in meta), SmartlingCoreExportApi::sendAttachmentForTranslation() (Gutenberg/Elementor images) and SmartlingCoreDownloadTrait::downloadTranslation().

With profile B chosen, children get profile A's credentials but B's job (JobEntityWithBatchUid::fromJob($submission->getJobInfo())), and existing B submissions are silently re-stamped to A. This was harmless while "active" meant "requested". Now that several profiles can be active, it isn't. Suggest: pass the parent's profile id into related-content creation, and in getSubmissionEntity() stamp only when getConfigurationProfileId() === null.

{
$curBlogId = $this->wordpressProxy->get_current_blog_id();
$profile = $this->settingsManager->getSingleSettingsProfile($curBlogId);
$profile = $this->settingsManager->resolveRequestedProfile($request->getProfileId(), $curBlogId);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[High] targetBlogIds aren't checked against the resolved profile's enabled target locales. A blog outside profile B (stale tab, crafted request, or a locale picked in the side widget, which now merges all profiles) is accepted. getSmartlingLocaleIdBySettingsProfile() then returns '', so the upload fails later, away from the request. Can we reject it here with a 400? The same applies to create-job (ContentEditJobController.php:161) and instant translation (InstantTranslationController.php:58).

Comment thread js/app.js
if (!widget) {
return;
}
widget.outerHTML = response.html;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[High] widget.outerHTML = response.html destroys the handlers smartling-connector-admin.js bound at page load. #smartling-download is bound directly (:196), and the checkbox handler is delegated from the widget node itself (:54). After the first refresh, Download does nothing and checkbox value syncing stops until the page is reloaded. Options: replace only the inner content and re-init the handlers, or move those handlers to $(document).on(...) delegation.

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']);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[High] CustomPostType::registerWidgetHandler() (CustomPostType.php:63) creates one controller per post type, and each registers this same wp_ajax_smartling_refresh_post_widget. The first registered controller handles every request and wp_send_json ends the request. Its preView() then filters submissions by its own servedContentType, so on a page or CPT the refreshed widget has no statuses. allReady never becomes true, so it polls all 5 times. Suggest registering the action once and using $post->post_type, or checking $post->post_type === $this->servedContentType and returning early so the matching controller can answer.

Comment thread js/app.js
throw new Error(submissionResponse.message?.global || 'Failed to add content to upload queue.');
}
setSuccess('Content successfully added to upload queue.');
if (!isBulkSubmitPage) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[High] !isBulkSubmitPage is also true on taxonomy term edit screens, which render the wizard (ContentEditJobController::box for WP_Term) and a #smartling-post-widget (taxonomy-based-content-type.php:61). The term id goes out as postId, so get_post(<termId>) either swaps in an unrelated post's widget or returns a 404 on every poll. Please limit polling to the post base type, for example by passing data-base-type to the wizard.

try {
$this->service->createSubmissions(UserTranslationRequest::fromArray($data));
$this->returnResponse(['status' => BaseAjaxServiceAbstract::RESPONSE_SUCCESS]);
} catch (SmartlingDbException $e) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Medium] SmartlingDbException is also thrown by DB, Queue, TaxonomyEntityStd, GravityFormsFormHandler, SmartlingCore and others, all reachable from createSubmissions(). Real DB or content errors will now show as "Invalid translation profile" and the original message is lost. It's also misleading when no profileId is sent and the blog has no active profile. Suggest a dedicated exception for profile resolution, or resolving the profile before this try block like the other two controllers do.

// that method, so it would otherwise keep whatever profile (or none) it had.
// Resubmitting an existing submission is an explicit new translation request, so restamp it with the
// profile it was requested with.
$submission->setConfigurationProfileId($profile->getId());

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Medium] (also applies to :110 and InstantTranslationController.php:323) Re-stamping an existing submission unconditionally means content that is IN_PROGRESS in project A and resubmitted under B loses its link to A's file and job. Status checks and download then go to B. Should we block or warn when the stored profile differs and the submission isn't completed or failed yet?

Comment thread js/app.js
action: 'smartling_job_api_proxy',
_wpnonce: nonce,
innerAction: 'list-jobs',
params: { profileId: selectedProfileId }

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Medium] On a profile switch the old job list stays selectable until the new one arrives (and stays forever if the request fails). selectedJob is cleared, but the user can still pick one of A's jobs and submit with profileId=B, which fails at createBatch. Suggest setJobs([]); setLoading(true); setError(''); at the start of the effect. Also, setError('Failed to load jobs') (:111) is never cleared after a later successful load.

Comment thread js/app.js
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;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Medium] The ticket says "One profile can be marked default for a blog; the user can override it." Here the default is profiles[0] (DB id order) overridden by per-browser localStorage. The server fallback is also "first active by id", as are cron (JobAbstract::getActiveProfile()), test run and the legacy widget upload. Is an admin-configurable default planned as a follow-up ticket?

*/
$locales = $data['profile']->getTargetLocales();
$locales = [];
foreach ($data['profiles'] ?? [$data['profile']] as $profile) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Medium] The side widget now lists locales from all profiles (first one wins per blog), but its legacy upload path ajaxUploadHandler() still uses ArrayHelper::first($this->getProfiles()) (PostBasedWidgetControllerStd.php:220). Choosing a blog that only belongs to profile 2 uploads through profile 1 with an empty Smartling locale. The "settings" link in the empty state (:150) also points only at the first profile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants