choose configuration profile UI (WP-1022)

[vsolovei-smartling]

No description provided.

[sl-mmuradov]

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.

[sl-mmuradov]

[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.

[vsolovei-smartling]

Fixed. SubmissionManager::getSubmissionEntity() now only stamps the source blog's default profile when none is set yet; an explicit profile id always wins. Threaded that through TranslationHelper (prepareSubmission/prepareSubmissionEntity/tryPrepareRelatedContent/getExistingSubmissionOrCreateNew), and ReferencedContentProcessor::processFieldPreTranslation() now passes the parent submission's own profile id.

Checked sendAttachmentForTranslation() and SmartlingCoreDownloadTrait::downloadTranslation(): neither has any caller anywhere in the codebase currently (grepped inc/) - the only live path creating related-content submissions is ReferencedContentProcessor during upload serialization. Added the optional profile id param to sendAttachmentForTranslation() too for signature consistency, but didn't invent a call site for downloadTranslation() since nothing reaches it today.

(5847c8d, tests in SubmissionManagerTest/ReferencedContentProcessorTest)

[sl-mmuradov]

[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).

[vsolovei-smartling]

Added SettingsManager::assertTargetBlogIdsBelongToProfile(), called from createSubmissions() (covers both the bulk and non-bulk paths), ContentEditJobController's create-job case, InstantTranslationController::handleRequestTranslation(), and the legacy widget's ajaxUploadHandler(). All four now reject with a 400 before doing any work instead of silently producing an empty Smartling locale.

(5847c8d)

[sl-mmuradov]

[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.

[vsolovei-smartling]

Fixed the live bug: the Download button's click handler in smartling-connector-admin.js is now delegated from document instead of bound directly to #smartling-download, so it survives the outerHTML swap.

Checked the checkbox-sync handler you mentioned (setCheckboxValue, bound in init()): init() only runs if ($(this.selectors.form).length > 0), and #smartling-form isn't rendered anywhere in the codebase (grepped inc/ and js/) - so that delegation was already dead code before this change, unrelated to the refresh. Left it alone rather than fixing an unrelated pre-existing gap in this PR.

(ab8892e)

[vsolovei-smartling]

Follow-up: the delegated-binding fix here was itself only a patch on a feature that kept causing problems (this, the multi-post-type collision, the taxonomy postId mismatch, and a nonce/referer duplication bug you found in the next round). Removed the whole widget auto-refresh instead of continuing to patch it - the Download button binding is back to the original direct $(selector).on(...), since there's no more outerHTML replacement for it to survive.

(50715fc)

[sl-mmuradov]

[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.

[vsolovei-smartling]

Fixed as you suggested: ajaxRefreshWidgetHandler() now returns early (without responding) when $post->post_type !== $this->servedContentType, letting the matching post type's instance answer instead.

(ab8892e)

[vsolovei-smartling]

Follow-up: removed ajaxRefreshWidgetHandler() and the smartling_refresh_post_widget action entirely rather than continuing to patch the auto-refresh feature (see the reply on the nonce-duplication comment from the latest round for why). This collision no longer applies since the endpoint doesn't exist anymore.

(50715fc)

[sl-mmuradov]

[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.

[vsolovei-smartling]

Added data-base-type to the #smartling-app container (post/taxonomy, from the same $baseType already computed in ContentEditJob.php) and gated the refresh call on baseType === 'post'. Bulk submit is unaffected (already gated on isBulkSubmitPage, and is always post-type content).

(ab8892e)

[vsolovei-smartling]

Follow-up: moot now - removed the widget auto-refresh feature entirely (the data-base-type gating along with it), so there's no more polling to misfire on taxonomy screens.

(50715fc)

[sl-mmuradov]

[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.

[vsolovei-smartling]

SettingsManager::resolveRequestedProfile() now throws SmartlingHumanReadableException instead of SmartlingDbException, and createSubmissionsHandler() catches that specifically (same pattern actionHandler() already used) before falling through to the generic Exception branch - so an unrelated SmartlingDbException now keeps its own message instead of being relabeled as an invalid profile.

(5847c8d, tests in ContentRelationsHandlerTest)

[sl-mmuradov]

[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?

[vsolovei-smartling]

Added a warning (not a block, since this is a resubmission the user explicitly asked for) when an existing submission is still IN_PROGRESS under a different profile - logs the submission id and old/new profile ids before the restamp. Same in InstantTranslationController::getOrCreateSubmission(). Happy to make this hard-block instead if that's the preference.

(5847c8d)

[sl-mmuradov]

[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.

[vsolovei-smartling]

Applied as suggested: setJobs([]); setLoading(true); setError(''); at the start of the effect.

(ab8892e)

[sl-mmuradov]

[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?

[vsolovei-smartling]

This was a deliberate scope decision made with the ticket owner before implementation, not an oversight: an admin-configurable per-blog default was discussed and intentionally dropped in favor of 'first active by id' plus a per-browser localStorage override, to avoid data-model and profile-management-UI changes for what's expected to be an edge case (most blogs only ever have one active profile). An admin-configurable default would be a reasonable follow-up ticket if it turns out to matter in practice.

[sl-mmuradov]

[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.

[vsolovei-smartling]

Fixed both. ajaxUploadHandler() now searches the blog's active profiles for the one whose enabled target locales actually cover every selected target blog (rejecting with a clear error if none does), instead of always using the first active one. The empty-state link now points at the profile list page instead of one specific profile's edit screen.

(5847c8d)

[vsolovei-smartling]

Addressed all four blocking issues and the three "should fix" items (replies inline on each comment; commits 5847c8d + ab8892e).

Summary:

• Related content created while processing a submission now inherits the parent submission's own profile instead of always falling back to the blog's default active profile.
• Target blog ids are now validated against the resolved profile's enabled locales in createSubmissions(), create-job, instant translation, and the legacy widget's upload handler - all reject with a 400 instead of silently producing an empty Smartling locale.
• Widget refresh: the Download button handler is now delegated from document so it survives being replaced, ajaxRefreshWidgetHandler() now defers to the controller instance whose post type actually matches, and the refresh call is gated to post base type so it doesn't misfire with a term id on taxonomy screens.
• ContentRelationsHandler now maps profile-resolution failures (SmartlingHumanReadableException) to their own key/message/code instead of mislabeling any SmartlingDbException as an invalid profile.
• Resubmitting a still-IN_PROGRESS submission under a different profile now logs a warning instead of silently losing its link to the previous project.
• A profile switch now clears the stale job list and any previous load error immediately, instead of leaving the old list selectable until (or unless) the new one arrives.
• The legacy widget's upload path now resolves the profile that actually covers the selected target blogs, and its empty-state settings link points at the profile list rather than one arbitrary profile.

Unit suite: 751/751 passing.

Tests added: SettingsManager::resolveRequestedProfile/assertTargetBlogIdsBelongToProfile, SubmissionManager's stamp-only-if-unset behavior, ReferencedContentProcessor's profile propagation, ContentRelationsHandler's exception mapping.

Not covered by new tests: the ajaxRefreshWidgetHandler() post-type-mismatch fix and the create-job 400 path - both are AJAX handlers that are hard to unit test in isolation here (no WP_Post stub in this suite, and ContentEditJobController's handler is an inline closure); the shared SettingsManager logic they call is covered directly. Didn't add Playwright coverage for the dropdown/reset-on-switch/widget-refresh flows in this pass either - open to doing that here or as a fast-follow.

[sl-mmuradov]

[High] ??= keeps the first profile's entry for each blog, even when that entry is disabled. Every profile stores a TargetLocale for every network blog, with an enabled flag (ConfigurationProfileFormController.php:239-256). So profile A's disabled entry for blog 3 wins over profile B's enabled one, and the isEnabled() filter below then drops it. In practice the side widget still shows only the first profile's enabled locales, so the multi-profile merge does nothing. Suggest merging only enabled locales:

foreach ($profile->getTargetLocales() as $locale) {
    if ($locale->isEnabled()) {
        $locales[$locale->getBlogId()] ??= $locale;
    }
}

[vsolovei-smartling]

Fixed - merge now only considers enabled locales:

if ($locale->isEnabled()) {
    $locales[$locale->getBlogId()] ??= $locale;
}

(50715fc)

[sl-mmuradov]

[Medium] preView() calls wp_nonce_field(self::WIDGET_NAME, self::CONNECTOR_NONCE) (:535) before the view prints #smartling-post-widget. That makes response.html the nonce input, plus a _wp_http_referer input, plus the widget. Because app.js:50 replaces only the widget's outerHTML, each poll (up to 5) adds another #smartling_connector_nonce and another _wp_http_referer input to the post edit form. The _wp_http_referer value is /wp-admin/admin-ajax.php, and because it comes later in the DOM it overrides the form's own referer on submit. Suggest rendering only the view here (pull the $this->view([...]) part out of preView()), or parsing the response in JS and swapping in only its #smartling-post-widget node.

[vsolovei-smartling]

Good catch, and it's a symptom of a bigger problem - the whole auto-refresh approach was fragile in ways that kept surfacing new issues (this one, the multi-post-type collision, the taxonomy postId mismatch). Rather than patch this one too, removed the refresh feature entirely: ajaxRefreshWidgetHandler(), the smartling_refresh_post_widget AJAX action, the frontend poller, and the plumbing that only existed to support it. The widget now only re-renders on normal page load, same as before this PR touched it.

(50715fc)

[sl-mmuradov]

[Medium] Choosing "the first active profile that covers all selected blogs" ignores which project $data['job']['id'] belongs to. If both A and B cover the selected blogs and the job is B's, this picks A. The submissions then get A's project id with B's job uid, and createBatch fails later. Also, nothing in js/ calls smartling_upload_handler any more, so this guessing code only serves external callers. Suggest accepting an explicit profileId and reusing resolveRequestedProfile() + assertTargetBlogIdsBelongToProfile() like the other three entry points do, instead of inferring it.

[vsolovei-smartling]

Fixed as suggested. ajaxUploadHandler() now resolves the profile via resolveRequestedProfile() (accepting an explicit profileId from $data, falling back to the blog's single active profile when none is sent) and validates target blogs with assertTargetBlogIdsBelongToProfile(), same as the other three entry points - no more inferring from which blogs happen to be covered.

(50715fc)

[sl-mmuradov]

[Medium] This is still open from the previous round ("re-fires all relation requests"). loadRelations depends on locales (:163), so a profile switch re-runs the depth-1 effect, and at depth 2 it also re-fetches every l1Relations entry. l1Relations/l2Relations are appended, never reset, so relations found for the old profile's target blogs stay listed and selectable, and totalRequests keeps growing, which skews the progress bar. Suggest resetting l1Relations, l2Relations, selectedRelations, pendingRequests and totalRequests here.

[vsolovei-smartling]

Fixed. handleProfileChange now also resets l1Relations, l2Relations, selectedRelations, pendingRequests and totalRequests, so the depth effect's re-fetch (triggered by loadRelations's identity changing with the new locales) starts clean instead of appending to the old profile's entries.

(50715fc)

[vsolovei-smartling]

Addressed the latest review round (commit 50715fc, replies inline):

• Removed the widget auto-refresh feature entirely rather than continuing to patch it - it kept surfacing new problems (the outerHTML handler-binding bug, the multi-post-type AJAX collision, the taxonomy postId mismatch, and now a duplicate nonce/referer-input bug from polling). ajaxRefreshWidgetHandler(), the smartling_refresh_post_widget action, the frontend poller, and all the plumbing that only existed to support it are gone. The side widget now only renders on normal page load, same as before this PR touched it. Posted follow-ups on the three earlier threads whose fixes this supersedes.
• Fixed the multi-profile locale merge in post-based-content-type.php to only consider enabled locales (a disabled entry from one profile was shadowing an enabled one from another).
• ajaxUploadHandler()'s legacy profile inference now resolves via resolveRequestedProfile()/assertTargetBlogIdsBelongToProfile() like the other three entry points, instead of guessing from which blogs happen to be covered.
• handleProfileChange now resets relation-tracking state (l1Relations, l2Relations, selectedRelations, pendingRequests, totalRequests) so switching profiles doesn't append to stale relations from the previous one.

Unit suite: 751/751 passing.