Skip to content

improve visual configurator for better elementor compatibility (WP-1018) #633

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
vsolovei-smartling merged 5 commits into from
Oct 1, 2026
Merged

improve visual configurator for better elementor compatibility (WP-1018) #633

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b0f9969
add widget scoping, conditions, preview and import/export to Visual C…
vsolovei-smartling Sep 29, 2026
0c03122
use distinct plugin id for Elementor 4 handler so logs are unambiguou…
vsolovei-smartling Sep 29, 2026
97d9071
address review comments on visual configurator rules (WP-1018)
vsolovei-smartling Sep 30, 2026
fbd20d6
replace ids for legacy related rules from the source, not the transla…
vsolovei-smartling Sep 30, 2026
84edb1e
address second round of review comments (WP-1018)
vsolovei-smartling Oct 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions inc/Smartling/ContentTypes/ExternalContentElementor3.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,12 @@ public function getMinVersion(): string
{
return '3';
}

/**
* Name used in logs to tell the handler apart from ExternalContentElementor4, the data key stays the plugin id
*/
public function getLogName(): string
{
return 'elementor3';
}
}
8 changes: 8 additions & 0 deletions inc/Smartling/ContentTypes/ExternalContentElementor4.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,12 @@ public function getMinVersion(): string
{
return '4';
}

/**
* Name used in logs to tell the handler apart from ExternalContentElementor3, the data key stays the plugin id
*/
public function getLogName(): string
{
return 'elementor4';

@sl-mmuradov sl-mmuradov Oct 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is more than a log label: ExternalContentManager (L39) stores the uploaded fields under getPluginId(), and ExternalContentElementorAbstract::setContentFields (L212) reads $translation[$this->getPluginId()].
Elementor 4 content uploaded before the upgrade has its strings under elementor. After the upgrade, download looks under elementor4, finds nothing, and the Elementor strings stay untranslated with no error. The Elementor 4 handler has been on master since WP-1000, so jobs can be in progress across the upgrade.
Suggest keeping elementor as the data key and changing only the log name, or falling back to $translation['elementor'] when elementor4 is missing.

@vsolovei-smartling vsolovei-smartling Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reworked to use dedicated log method

}
}
125 changes: 113 additions & 12 deletions inc/Smartling/ContentTypes/ExternalContentJsonRules.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use Smartling\Submissions\SubmissionEntity;
use Smartling\Tuner\JsonFieldRule;
use Smartling\Tuner\JsonFieldRulesManager;
use Smartling\Tuner\JsonLeafMatcher;
use Smartling\Vendor\JsonPath\JsonObject;

class ExternalContentJsonRules implements ContentTypeModifyingInterface
Expand All @@ -17,11 +18,15 @@ class ExternalContentJsonRules implements ContentTypeModifyingInterface

public const PLUGIN_ID = 'json-rules';

private JsonLeafMatcher $matcher;

public function __construct(
private JsonFieldRulesManager $rulesManager,
private ReplacerFactory $replacerFactory,
private WordpressFunctionProxyHelper $wpProxy,
?JsonLeafMatcher $matcher = null,
) {
$this->matcher = $matcher ?? new JsonLeafMatcher();
}

public function getMaxVersion(): string
Expand All @@ -39,6 +44,11 @@ public function getPluginId(): string
return self::PLUGIN_ID;
}

public function getLogName(): string
{
return self::PLUGIN_ID;
}

public function getPluginPaths(): array
{
return [];
Expand Down Expand Up @@ -75,6 +85,14 @@ public function getContentFields(SubmissionEntity $submission, bool $raw): array
if ($this->parseReplacer($rule->getReplacerId())[0] !== ReplacerFactory::REPLACER_TRANSLATE) {
continue;
}
if ($rule->isExtended()) {
foreach ($this->matcher->match($json, $rule) as $leaf) {
if (is_string($leaf['value']) && $leaf['value'] !== '') {
$result[$this->buildLeafKey($metaKey, $leaf['segments'])] = $leaf['value'];
}
}
continue;
}
$matches = $this->safeGet($json, $rule->getPropertyPath());
foreach ($matches as $index => $value) {
if (is_string($value) && $value !== '') {
Expand All @@ -86,6 +104,16 @@ public function getContentFields(SubmissionEntity $submission, bool $raw): array
return $result;
}

/**
* @return array<int, mixed> values matched by the rule in document order, used for previews
*/
public function getMatchedValues(array $json, JsonFieldRule $rule): array
{
return $rule->isExtended()
? array_column($this->matcher->match($json, $rule), 'value')
: array_values($this->safeGet($json, $rule->getPropertyPath()));
}

public function getRelatedContent(string $contentType, int $contentId): array
{
$result = [];
Expand All @@ -101,7 +129,10 @@ public function getRelatedContent(string $contentType, int $contentId): array
continue;
}
$referencedType = $hint !== '' ? $hint : ContentTypeHelper::CONTENT_TYPE_UNKNOWN;
foreach ($this->safeGet($json, $rule->getPropertyPath()) as $value) {
$values = $rule->isExtended()
? array_column($this->matcher->match($json, $rule), 'value')
: $this->safeGet($json, $rule->getPropertyPath());
foreach ($values as $value) {
if (is_numeric($value) && (int)$value > 0) {
$result[$referencedType][] = (int)$value;
}
Expand Down Expand Up @@ -134,13 +165,16 @@ public function setContentFields(array $original, array $translation, Submission
$this->getLogger()->debug("Failed to parse meta $metaKey as JSON: " . $e->getMessage());
continue;
}
// Extended rules are matched against the source, not the (possibly already translated) copy being edited:
// conditions on translatable siblings must see the same values as they did on upload
$matchSource = $this->decodeJson($original['meta'][$metaKey] ?? null) ?? $jsonObject->getValue();
$modified = false;
foreach ($rules as $rule) {
[$replacer] = $this->parseReplacer($rule->getReplacerId());
if ($replacer === ReplacerFactory::REPLACER_TRANSLATE) {
$modified = $this->applyTranslateRule($jsonObject, $rule, $metaKey, $translations) || $modified;
$modified = $this->applyTranslateRule($jsonObject, $matchSource, $rule, $metaKey, $translations) || $modified;
} elseif ($replacer === ReplacerFactory::REPLACER_RELATED) {
$modified = $this->applyRelatedRule($jsonObject, $rule, $submission) || $modified;
$modified = $this->applyRelatedRule($jsonObject, $matchSource, $rule, $submission) || $modified;
}
}
if ($modified) {
Expand Down Expand Up @@ -203,8 +237,35 @@ private function safeGet(array $json, string $path): array
return $result;
}

private function applyTranslateRule(JsonObject $jsonObject, JsonFieldRule $rule, string $metaKey, array $translations): bool
private function decodeJson(mixed $value): ?array
{
if (!is_string($value) || $value === '') {
return null;
}
try {
$decoded = json_decode($value, true, 512, JSON_THROW_ON_ERROR);
} catch (\JsonException) {
return null;
}

return is_array($decoded) ? $decoded : null;
}

private function applyTranslateRule(JsonObject $jsonObject, array $matchSource, JsonFieldRule $rule, string $metaKey, array $translations): bool
{
if ($rule->isExtended()) {
$data = &$jsonObject->getValue();
$changed = false;
foreach ($this->matcher->match($matchSource, $rule) as $leaf) {
$key = $this->buildLeafKey($metaKey, $leaf['segments']);
if (array_key_exists($key, $translations)) {
$changed = $this->matcher->setValue($data, $leaf['segments'], $translations[$key]) || $changed;
}
}
unset($data);

return $changed;
}
$objects = $jsonObject->getJsonObjects($rule->getPropertyPath());
if ($objects === false || $objects === null) {
return false;
Expand All @@ -225,25 +286,44 @@ private function applyTranslateRule(JsonObject $jsonObject, JsonFieldRule $rule,
return $changed;
}

private function applyRelatedRule(JsonObject $jsonObject, JsonFieldRule $rule, SubmissionEntity $submission): bool
private function applyRelatedRule(JsonObject $jsonObject, array $matchSource, JsonFieldRule $rule, SubmissionEntity $submission): bool
{
try {
$replacer = $this->replacerFactory->getReplacer($rule->getReplacerId());
} catch (\Throwable $e) {
$this->getLogger()->notice("Unable to resolve replacer {$rule->getReplacerId()}: " . $e->getMessage());
return false;
}
$objects = $jsonObject->getJsonObjects($rule->getPropertyPath());
if ($objects === false || $objects === null) {
return false;
if ($rule->isExtended()) {
$data = &$jsonObject->getValue();
$changed = false;
foreach ($this->matcher->match($matchSource, $rule) as $leaf) {
// Replace from the source id: ids another handler already remapped in the copy must not be remapped
// again, a target id can collide with an unrelated source id in multisite
$original = $leaf['value'];
if (!is_numeric($original) || (int)$original <= 0) {
continue;
}
$replaced = $replacer->processAttributeOnDownload($original, $original, $submission);
if ($replaced !== $original) {
$changed = $this->matcher->setValue($data, $leaf['segments'], $replaced) || $changed;
}
}
unset($data);

return $changed;
}
if (!is_array($objects)) {
$objects = [$objects];
$objects = $this->getNodes($jsonObject, $rule->getPropertyPath());
// Read ids from the source, the copy may already hold ids remapped by another handler (see extended branch)
$sourceObjects = $this->getNodes(new JsonObject($matchSource), $rule->getPropertyPath());
$usesSource = count($sourceObjects) === count($objects);
if (!$usesSource) {
$this->getLogger()->notice("Source and translation differ in structure for path={$rule->getPropertyPath()}, using translated values");
}
$changed = false;
foreach ($objects as $node) {
foreach ($objects as $index => $node) {
$ref = &$node->getValue();
$original = $ref;
$original = $usesSource ? $sourceObjects[$index]->getValue() : $ref;
if (!is_numeric($original) || (int)$original <= 0) {
unset($ref);
continue;
Expand All @@ -258,6 +338,19 @@ private function applyRelatedRule(JsonObject $jsonObject, JsonFieldRule $rule, S
return $changed;
}

/**
* @return JsonObject[]
*/
private function getNodes(JsonObject $jsonObject, string $path): array
{
$objects = $jsonObject->getJsonObjects($path);
if ($objects === false || $objects === null) {
return [];
}

return is_array($objects) ? array_values($objects) : [$objects];
}

/**
* @return array{0:string,1:string} [replacerId, contentTypeHint]
*/
Expand All @@ -267,6 +360,14 @@ private function parseReplacer(string $replacerId): array
return [$parts[0], $parts[1] ?? ''];
}

/**
* @param array<int, string|int> $segments
*/
private function buildLeafKey(string $metaKey, array $segments): string
{
return $metaKey . '|' . $this->matcher->pathToString($segments);
}

private function buildKey(string $metaKey, string $path, int $index): string
{
return $metaKey . '|' . $path . '|' . $index;
Expand Down
20 changes: 10 additions & 10 deletions inc/Smartling/ContentTypes/ExternalContentManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,12 @@ public function getExternalContent(array $source, SubmissionEntity $submission,
return $this->siteHelper->withBlog($submission->getSourceBlogId(), function () use ($raw, $source, $submission) {
foreach ($this->handlers as $handler) {
if ($handler->getSupportLevel($submission->getContentType(), $submission->getSourceId()) === Pluggable::SUPPORTED) {
$this->getLogger()->debug("Determined support for {$handler->getPluginId()}, will try to get fields");
$this->getLogger()->debug("Determined support for {$handler->getLogName()}, will try to get fields");
try {
$submission->assertHasSource();
$source[$handler->getPluginId()] = $handler->getContentFields($submission, $raw);
} catch (\Throwable $e) {
$this->getLogger()->notice('HandlerName="' . $handler->getPluginId() . '" got exception while trying to get external content: ' . $e->getMessage());
$this->getLogger()->notice('HandlerName="' . $handler->getLogName() . '" got exception while trying to get external content: ' . $e->getMessage());
}
}
if ($handler instanceof ContentTypeModifyingInterface) {
Expand All @@ -47,10 +47,10 @@ public function getExternalContent(array $source, SubmissionEntity $submission,
$source = $handler->removeUntranslatableFieldsForUpload($source, $submission);
$count = count($this->fieldsFilterHelper->flattenArray($source));
if ($previousCount !== $count) {
$this->getLogger()->info('HandlerName="' . $handler->getPluginId() . '" altered content fields for upload, previousCount=' . $previousCount . ', count=' . $count);
$this->getLogger()->info('HandlerName="' . $handler->getLogName() . '" altered content fields for upload, previousCount=' . $previousCount . ', count=' . $count);
}
} catch (\Throwable $e) {
$this->getLogger()->warning('HandlerName="' . $handler->getPluginId() . '" got exception while trying to alter content fields: ' . $e->getMessage());
$this->getLogger()->warning('HandlerName="' . $handler->getLogName() . '" got exception while trying to alter content fields: ' . $e->getMessage());
}
}
}
Expand All @@ -74,11 +74,11 @@ public function getExternalRelations(string $contentType, int $id): array
$result = [];
foreach ($this->handlers as $handler) {
if ($handler->getSupportLevel($contentType, $id) === Pluggable::SUPPORTED) {
$this->getLogger()->debug("Determined support for {$handler->getPluginId()}, will try to get related content");
$this->getLogger()->debug("Determined support for {$handler->getLogName()}, will try to get related content");
try {
$result = array_merge_recursive($result, $handler->getRelatedContent($contentType, $id));
} catch (\Throwable $e) {
$this->getLogger()->notice('HandlerName="' . $handler->getPluginId() .
$this->getLogger()->notice('HandlerName="' . $handler->getLogName() .
'" got errorClass="' . $e::class . '" while trying to get external related content: ' .
$e->getMessage());
}
Expand All @@ -100,18 +100,18 @@ public function setExternalContent(array $original, array $translation, Submissi
{
foreach ($this->handlers as $handler) {
if ($handler->getSupportLevel($submission->getContentType(), $submission->getSourceId()) === Pluggable::SUPPORTED) {
$this->getLogger()->debug("Determined support for {$handler->getPluginId()}, will try to set fields");
$this->getLogger()->debug("Determined support for {$handler->getLogName()}, will try to set fields");
try {
$externalContent = $handler->setContentFields($original, $translation, $submission);
if ($externalContent !== null) {
$this->getLogger()->info('Content array modified by HandlerName="' . $handler->getPluginId() . '"');
$this->getLogger()->info('Content array modified by HandlerName="' . $handler->getLogName() . '"');
$translation = $externalContent;
}
} catch (\Throwable $e) {
$this->getLogger()->notice('HandlerName="' . $handler->getPluginId() . '" got exception while trying to set external content: ' . $e->getMessage());
$this->getLogger()->notice('HandlerName="' . $handler->getLogName() . '" got exception while trying to set external content: ' . $e->getMessage());
}
} else {
$this->getLogger()->debug("No support for {$handler->getPluginId()} detected");
$this->getLogger()->debug("No support for {$handler->getLogName()} detected");
}
}

Expand Down
8 changes: 8 additions & 0 deletions inc/Smartling/Extensions/Pluggable.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,16 @@ public function getMaxVersion(): string;

public function getMinVersion(): string;

/**
* Key under which the handler's data is stored in submissions, must stay stable across versions
*/
public function getPluginId(): string;

/**
* Name to use in logs, may differ from the plugin id when handlers share it
*/
public function getLogName(): string;

/**
* @return array with possible paths to the plugin file relative to the plugins directory.
* Most plugins have a single possible path, some have variations.
Expand Down
5 changes: 5 additions & 0 deletions inc/Smartling/Extensions/PluggableAbstract.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ public function __construct(protected PluginHelper $pluginHelper, protected Word
{
}

public function getLogName(): string
{
return $this->getPluginId();
}

public function getPluginSupportLevel(): string
{
$result = Pluggable::NOT_SUPPORTED;
Expand Down
Loading
Loading