add filter for target content id, improve elementor related ids detection (WP-1020) by vsolovei-smartling · Pull Request #637 · Smartling/wordpress-localization-plugin

vsolovei-smartling · 2026-10-07T08:54:49Z

No description provided.

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

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

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

…y (WP-1020) - TargetIdFilter: tolerate any argument type, never throw, cache lookups in the object cache, log misses and ambiguity at notice, return invalid source id unchanged when falling back - Elementor query exclude_term_ids/exclude_ids are remapped to translated ids but no longer submitted for translation (Content::isRemapOnly) - validate Elementor query ids as integers, move loop widget related content into ElementorQueryRelated::addLoopRelated - add tests Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

sl-mmuradov

Thanks, the direction looks good. Replacing the trait with the composed ElementorQueryRelated removes the duplicated code in Posts and Loop Carousel, and the remap-only flag on Content is small and backward compatible.

Blocking

Term ID type (please verify): as far as I know, Elementor Pro stores query term IDs as term_taxonomy_ids, while we resolve and remap them as term_ids. On sites where they differ, this submits the wrong terms and writes broken IDs back, which is the WP-1020 symptom itself. This existed before for include_term_ids, but the PR extends it to more settings and widgets.
Leftover query settings: IDs are collected regardless of the widget's query mode. Since posts_ids is now submitted, an old hidden manual selection can queue many posts. Please check {prefix}post_type / {prefix}include / {prefix}exclude first.

Should fix

normalizeReferences() overwrites taxonomy references instead of merging, which drops detected query terms when the document has its own terms in that taxonomy. Fix here or file a follow-up.
Document that excluded IDs are only remapped if the content is already translated when the translation is applied.

Notes on code not changed by this PR

inc/Smartling/Services/ContentRelationsDiscoveryService.php:576-578: $result[$taxonomy] = $ids; overwrites the result instead of merging. If an Elementor document has terms of its own in a taxonomy (e.g. a post with categories), any query terms from the same taxonomy that this PR detects are dropped from the related list. Suggest $result[$taxonomy] = array_merge($result[$taxonomy] ?? [], $ids);, either here or in a follow-up ticket.
inc/Smartling/ContentTypes/Elementor/ElementAbstract.php:138: for query settings, getTerm() here receives a term_taxonomy_id, not a term_id (see the comment on ElementorQueryRelated.php:22).
tests/Mocks/WordpressFunctionsMockHelper.php:212: term_taxonomy_id => 0 means the tests can't tell term_id and term_taxonomy_id apart. Worth giving it a different value once the ID type is handled.

Note: I couldn't run the test suite locally, so this review is based on reading the code.

sl-mmuradov · 2026-10-08T09:21:45Z

+{
+    use LoggerSafeTrait;
+
+    private const TERM_ID_SUFFIXES = ['include_term_ids' => false, 'exclude_term_ids' => true];


As far as I remember, Elementor Pro stores *_include_term_ids / *_exclude_term_ids as term_taxonomy_ids, not term_ids. Elementor_Post_Query::build_terms_query() resolves them with get_term_by('term_taxonomy_id', $id). Could you check this against the Elementor Pro source?

If that's right, this list is resolved wrongly downstream:

ElementAbstract::setRelations() (line 138) and ContentRelationsDiscoveryService::normalizeReferences() both call getTerm($id), which takes a term_id.

Submissions are matched by source_id, which is a term_id.

On sites where the two IDs differ (older or multisite installs, terms that were split), we submit the wrong term, and on download we write a target term_id where Elementor expects a target term_taxonomy_id. That's the symptom described in WP-1020. Converting tt_id → term_id on the way in and term_id → tt_id when writing back should fix it.

Confirmed against Elementor Pro 3.33.2 (modules/query-control/classes/elementor-post-query.php): build_terms_query() resolves the IDs with get_term_by( 'term_taxonomy_id', $id ) and builds the tax_query with 'field' => 'term_taxonomy_id'.

Fixed in faf5950: the query term lists are flagged with Content::isTermTaxonomyId(). ExternalContentElementorAbstract::getData() converts them to term_ids in the source blog (so discovery, submissions and the taxonomy lookup in setRelations() all work with term_ids), and setRelations() converts the translated term_id back to its term_taxonomy_id in the target blog before writing. Terms that cannot be resolved are dropped and logged instead of being submitted as a different term. The mock in WordpressFunctionsMockHelper now returns a term_taxonomy_id different from term_id, and there are handler tests for both directions. Documented in ELEMENTOR_DEVELOPMENT.md.

Only unit tests so far, I have not run it against a multisite where the two IDs differ.

sl-mmuradov · 2026-10-08T09:21:45Z

+    use LoggerSafeTrait;
+
+    private const TERM_ID_SUFFIXES = ['include_term_ids' => false, 'exclude_term_ids' => true];
+    private const POST_ID_SUFFIXES = ['posts_ids' => false, 'exclude_ids' => true];


These IDs are collected no matter which query mode the widget uses. Elementor keeps values of controls that are hidden, but only uses them when the mode matches:

posts_ids only when {prefix}post_type === 'by_id'

include_term_ids only when {prefix}include contains terms

exclude_ids / exclude_term_ids only when {prefix}exclude contains manual_selection / terms

Collecting posts_ids is new in this PR. A widget that once had a manual selection and was then switched to "by category" will now submit all of those posts as related content, each with its own related content. Could we check the mode settings before adding the IDs?

Agreed, and the source is stricter than the list in the review: with post_type = by_id only posts_ids is used, and the exclude, avoid-duplicates, terms and author args are skipped entirely. Otherwise posts_ids is ignored and the other lists need their mode to contain the expected value.

Fixed in 40e4c58 and 347aaee: ElementorQueryRelated now collects posts_ids only for by_id, and include_term_ids / exclude_term_ids / exclude_ids only when the mode is not by_id and {prefix}include / {prefix}exclude contain terms / terms / manual_selection. Tests cover both modes and leftover values; the mode table is in ELEMENTOR_DEVELOPMENT.md. On the data many widgets carry such leftovers (for example exclude_ids in by_id mode and posts_ids when the type is not by_id).

sl-mmuradov · 2026-10-08T09:21:45Z

        $return = [];
        foreach ($flat as $item) {
            assert($item instanceof Content);
+            if ($item->isRemapOnly()) {


Remap-only IDs are only replaced if the content is already translated when the translation is applied. If the excluded term or post is translated later, the page keeps the source ID until it's downloaded again. That's fine as a trade-off, but please document it in ELEMENTOR_DEVELOPMENT.md and the release notes so support knows what to expect.

Documented in ELEMENTOR_DEVELOPMENT.md (Pattern 5, "Excluded content is remap-only") and added to the 5.8.1 entry in the readme.txt changelog: excluded IDs are replaced only if the content is already translated when the page translation is applied; if it is translated later, the page has to be downloaded again.

…rge taxonomy references, document remap-only (WP-1020) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

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

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

vsolovei-smartling · 2026-10-08T18:45:31Z

Addressed in 40e4c58, 347aaee and faf5950:

Term ID type: query term lists are treated as term_taxonomy_ids and converted to term_id and back (verified against Elementor Pro 3.33.2).
Leftover query settings: IDs are collected only for the active query mode.
normalizeReferences() overwrite: now merges (array_merge), with a test that fails without the fix.
Remap-only caveat: documented in ELEMENTOR_DEVELOPMENT.md and the changelog.
Mock: term_taxonomy_id now differs from term_id.

Unit tests pass (754); not yet verified on a multisite where the IDs differ.

sl-mmuradov · 2026-10-09T07:49:54Z

+
+    public function addRelated(RelatedContentInfo $info, array $settings, string $containerId, string $prefix): RelatedContentInfo
+    {
+        $isManualSelection = ($settings[$prefix . 'post_type'] ?? null) === self::MANUAL_SELECTION;


High: $isManualSelection only separates by_id from every other mode, so current_query (and possibly related) still collects leftover IDs.

As far as I remember, Elementor_Post_Query::get_query_args() returns early for {prefix}post_type === 'current_query': it uses the main $wp_query vars and ignores include/exclude completely. A widget switched from "by category" to "Current Query" still has include = ['terms'] and the term IDs saved, so those terms would be submitted as related content. That's the same leftover problem as before, in another mode.

Could you check current_query against the 3.33.2 source you used? Please also check related (Elementor_Related_Query), which I think ignores include_term_ids. If confirmed, skip everything for current_query, add the mode to the table in ELEMENTOR_DEVELOPMENT.md, and add a case to testIdsOfInactiveQueryModesAreIgnored.

Confirmed against Elementor Pro 3.33.2:

current_query: Elementor_Post_Query::get_query_args() returns the main $wp_query->query_vars right away, so none of the include/exclude/ID settings is used.

related: Elementor_Related_Query overrides build_terms_query_include() and takes the terms of the current post (related_taxonomies, tt_ids), so include_term_ids is ignored (the control is also hidden for it). exclude_term_ids and exclude_ids still apply, so those are kept.

Fixed in 8537b2d: everything is skipped for current_query, only include_term_ids is skipped for related. The mode table in ELEMENTOR_DEVELOPMENT.md is now by post_type, testIdsOfInactiveQueryModesAreIgnored has the current-query case, and there is a separate related test.

sl-mmuradov · 2026-10-09T07:49:55Z

@@ -63,6 +63,9 @@ Additional information on the Smartling Connector for WordPress can be found [he

 == Changelog ==
 = 5.8.0 =


Medium (blocks merge until confirmed): following up on your note about moving the bullets. The merge base 804a95d3 already set Stable tag: 5.8.0 and Version: 5.8.0 in smartling-connector.php. If 5.8.0 has been published, these three bullets need a new version entry (5.8.1 / 5.9.0), plus the version bump in smartling-connector.php and composer.json.

5.8.0 is published, so the entries moved to a new 5.8.1 section and I bumped the version in smartling-connector.php, composer.json and Stable tag in readme.txt (3d139b5). The 5.8.0 changelog entry is back to what was published. I also added a bullet for the numeric-key fix below.

sl-mmuradov · 2026-10-09T07:49:55Z

        }

-        return new ExternalData(strings: $strings, relatedContentInfo: $related);
+        return new ExternalData(strings: $strings, relatedContentInfo: $this->convertTermTaxonomyIds($related, $sourceBlogId));


Medium (existing code, but it affects the new query IDs): getData() combines each top-level element's related info with RelatedContentInfo::merge() (RelatedContentInfo.php:126). That uses array_merge(), which renumbers numeric keys. arrayMergePreserveKeys() exists because Elementor sometimes generates all-digit element IDs. A top-level container with such an ID loses its key, so setTargetContent() finds nothing at $info->getInfo()[$this->id] and none of the query IDs under it are remapped.

Suggest $result->info = $this->arrayMergePreserveKeys($result->info, $info->info); in merge(), here or in a follow-up ticket.

Good catch. RelatedContentInfo::merge() now uses arrayMergePreserveKeys() (8537b2d), so all-digit container IDs keep their keys. ExternalData also uses merge() and benefits from it. The new test fails without the change. Added to the 5.8.1 changelog.

sl-mmuradov · 2026-10-09T07:49:55Z

+            $targetId = max(self::NOT_FOUND, current($submissions)->getTargetId());
+        }
+        if ($targetId === self::NOT_FOUND) {
+            $this->getLogger()->notice(count($submissions) > 1


Medium: this filter runs from theme/snippet code on front-end page views. Without a persistent object cache, WpObjectCache lasts one request. That means one find() query and one notice per untranslated ID per page view, and the logger runs at DEBUG by default (inc/config/logger.yml:6). A missing translation is expected here (the filter called on the source blog, content that isn't submitted). Suggest debug, or keep notice only for the ambiguous-match case.

Agreed. A missing translation is now logged at debug; notice stays only for the ambiguous-match case (8537b2d).

…reserve numeric keys on merge, log missing target id at debug (WP-1020) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

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

vsolovei-smartling and others added 5 commits October 6, 2026 13:19

treat Elementor query term/post IDs as related content, add loop-grid…

abc9cdd

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

add smartling_target_id filter to translate ids in custom code (WP-1020)

c31ef21

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

default smartling_target_id fallbackToSource to true (WP-1020)

d411f3d

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

replace ElementorQueryRelatedTrait with composed ElementorQueryRelate…

d6e1cbe

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

vsolovei-smartling requested review from PavelLoparev and sl-mmuradov October 8, 2026 08:59

sl-mmuradov suggested changes Oct 8, 2026

View reviewed changes

vsolovei-smartling and others added 4 commits October 8, 2026 14:39

update claude.md

00e32dc

address PR #637 review: collect only active Elementor query modes, me…

40e4c58

…rge taxonomy references, document remap-only (WP-1020) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

ignore Elementor query term lists and exclude_ids in manual selection…

347aaee

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

convert Elementor query term_taxonomy_ids to term_ids and back (WP-1020)

faf5950

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

vsolovei-smartling requested a review from sl-mmuradov October 8, 2026 18:45

sl-mmuradov reviewed Oct 9, 2026

View reviewed changes

vsolovei-smartling and others added 2 commits October 9, 2026 15:02

address PR #637 round 2: skip current_query/related query settings, p…

8537b2d

…reserve numeric keys on merge, log missing target id at debug (WP-1020) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

bump version to 5.8.1, move query changelog entries (WP-1020)

3d139b5

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

vsolovei-smartling requested a review from sl-mmuradov October 9, 2026 13:08

sl-mmuradov approved these changes Oct 9, 2026

View reviewed changes

vsolovei-smartling merged commit 7849464 into master Oct 10, 2026
3 checks passed

vsolovei-smartling deleted the WP-1020-elementor-queries-related-content branch October 10, 2026 09:02

		@@ -63,6 +63,9 @@ Additional information on the Smartling Connector for WordPress can be found [he

		== Changelog ==
		= 5.8.0 =

Conversation

vsolovei-smartling commented Oct 7, 2026

Uh oh!

sl-mmuradov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vsolovei-smartling Oct 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vsolovei-smartling Oct 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vsolovei-smartling commented Oct 8, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vsolovei-smartling Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

vsolovei-smartling Oct 8, 2026 •

edited

Loading

vsolovei-smartling Oct 8, 2026 •

edited

Loading

vsolovei-smartling Oct 9, 2026 •

edited

Loading