-
Notifications
You must be signed in to change notification settings - Fork 5
fix issue (WP-834) Elementor content partially missing upon translation #489
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.
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
fix issue (WP-834) Elementor content partially missing upon translation #489
Conversation
| logger.filehandler.standard.maxfiles: 1 | ||
|
|
||
| logger.formatter.date: "Y-m-d\TH:i:s.up" | ||
| logger.formatter.date: "Y-m-d\\TH:i:s.up" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seen as error in logs, originally thought this caused the issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need this slash at all here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The parser library triggers an error (E_USER_ERROR), as per their requirements, doubly quoted strings must escape the slash. It ends up in logs, but WordPress is not down, as the log entries state
| } | ||
| if (is_array($setting)) { | ||
| if (array_key_exists('id', $setting) && array_key_exists('url', $setting) && is_int($setting['id'])) { | ||
| if (array_key_exists('id', $setting) && array_key_exists('url', $setting) && is_int($setting['id']) && ($setting['source'] ?? '') === 'library') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensured only attachments from WP media library get replaced
| private function getRelatedFromSetting(array $setting): ?array { | ||
| if ($setting['source'] ?? '' === 'library' && array_key_exists('id', $setting)) { | ||
| return [ContentTypeHelper::POST_TYPE_ATTACHMENT => [$setting['id'] => $setting['id']]]; | ||
| if (($setting['source'] ?? '') === 'library' && ($setting['id'] ?? '') !== '') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Real cause of the issue, empty ids were added to related content discovery service, preventing correct functioning
| if ($setting['source'] ?? '' === 'library' && array_key_exists('id', $setting)) { | ||
| return [ContentTypeHelper::POST_TYPE_ATTACHMENT => [$setting['id'] => $setting['id']]]; | ||
| if (($setting['source'] ?? '') === 'library' && ($setting['id'] ?? '') !== '') { | ||
| return [ContentTypeHelper::POST_TYPE_ATTACHMENT => [$setting['id']]]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Return format made consistent with other external content providers
| * License URI: http://www.gnu.org/licenses/gpl-2.0.txt | ||
| * Text Domain: smartling-connector | ||
| * Domain Path: /languages | ||
| * Requires PHP: 8.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not issue related, but when performing manual updates WP gets information from plugin php file instead of readme
| $submissionManager->expects($this->exactly(2))->method('findOne')->withConsecutive([[ | ||
| SubmissionEntity::FIELD_CONTENT_TYPE => ContentTypeHelper::POST_TYPE_ATTACHMENT, | ||
| SubmissionEntity::FIELD_SOURCE_BLOG_ID => $sourceBlogId, | ||
| SubmissionEntity::FIELD_TARGET_BLOG_ID => $targetBlogId, | ||
| SubmissionEntity::FIELD_SOURCE_ID => $sourceAttachmentId, | ||
| ]], [[ | ||
| SubmissionEntity::FIELD_CONTENT_TYPE => ExternalContentElementor::CONTENT_TYPE_ELEMENTOR_LIBRARY, | ||
| SubmissionEntity::FIELD_SOURCE_BLOG_ID => $sourceBlogId, | ||
| SubmissionEntity::FIELD_TARGET_BLOG_ID => $targetBlogId, | ||
| SubmissionEntity::FIELD_SOURCE_ID => $sourceWidgetId, | ||
| ]])->willReturnOnConsecutiveCalls($foundSubmissionAttachment, $foundSubmissionWidget); | ||
| $matcher = $this->exactly(3); | ||
| $submissionManager->expects($matcher)->method('findOne')->willReturnCallback( | ||
| function ($value) use ($foundSubmissionAttachment, $foundSubmissionBackground, $foundSubmissionWidget, $matcher, $sourceAttachmentId, $sourceBackgroundId, $sourceBlogId, $sourceWidgetId, $targetBlogId) { | ||
| switch ($matcher->getInvocationCount()) { | ||
| case 1: | ||
| $this->assertEquals([ | ||
| SubmissionEntity::FIELD_CONTENT_TYPE => ContentTypeHelper::POST_TYPE_ATTACHMENT, | ||
| SubmissionEntity::FIELD_SOURCE_BLOG_ID => $sourceBlogId, | ||
| SubmissionEntity::FIELD_TARGET_BLOG_ID => $targetBlogId, | ||
| SubmissionEntity::FIELD_SOURCE_ID => $sourceAttachmentId, | ||
| ], $value); | ||
|
|
||
| return $foundSubmissionAttachment; | ||
| case 2: | ||
| $this->assertEquals([ | ||
| SubmissionEntity::FIELD_CONTENT_TYPE => ContentTypeHelper::POST_TYPE_ATTACHMENT, | ||
| SubmissionEntity::FIELD_SOURCE_BLOG_ID => $sourceBlogId, | ||
| SubmissionEntity::FIELD_TARGET_BLOG_ID => $targetBlogId, | ||
| SubmissionEntity::FIELD_SOURCE_ID => $sourceBackgroundId, | ||
| ], $value); | ||
|
|
||
| return $foundSubmissionBackground; | ||
| case 3: | ||
| $this->assertEquals([ | ||
| SubmissionEntity::FIELD_CONTENT_TYPE => ExternalContentElementor::CONTENT_TYPE_ELEMENTOR_LIBRARY, | ||
| SubmissionEntity::FIELD_SOURCE_BLOG_ID => $sourceBlogId, | ||
| SubmissionEntity::FIELD_TARGET_BLOG_ID => $targetBlogId, | ||
| SubmissionEntity::FIELD_SOURCE_ID => $sourceWidgetId, | ||
| ], $value); | ||
|
|
||
| return $foundSubmissionWidget; | ||
| } | ||
| throw new \LogicException('Unexpected invocation'); | ||
| } | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added test case for background image and removed deprecated withConsecutive
|
|
||
| class ExternalDataTest extends TestCase { | ||
|
|
||
| public function testAddRelated() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added expectations for external content providers as test
| logger.filehandler.standard.maxfiles: 1 | ||
|
|
||
| logger.formatter.date: "Y-m-d\TH:i:s.up" | ||
| logger.formatter.date: "Y-m-d\\TH:i:s.up" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need this slash at all here?
Co-authored-by: Dmitry Studynsky <dimitrystd@users.noreply.github.com>
No description provided.