-
Notifications
You must be signed in to change notification settings - Fork 13
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
Bug/371/seo image missing from draft #372
Conversation
* fix: craft 4 changes * fix: user access, postaction request and new translation button * fix: translator page fixes * fix: static translation index page and order details actions * fix: order detail actions and index exporter * fix: element actions on order index page * fix: dashboard widgets * fix: prevent target updated when status is modified * fix: craft 4 code refactor * fix: where table comparison not available on publish modal * fix: new translation button remain active while switching entry groups in entry index page * fix: apply translation draft from entry detail page and unused code removal * fix: assets, category and global sets drafts * fix: static translations download issue and category draft table migration removed * fix: static translation saved notice * fix: order from jobs dont reload page after job finished * fix: minor issues found in testing * fix: min version required in setting check page * update: composer.json file for min craft version * fix: desc icon issue fix * fix: remove draftId and dateDelivered when draft is deleted, ignore target chnges for canceled order, check for order status changes when draft deleted * fix: global set delete draft from detail page removing file row issue * fix: minor testing bugs * fix: update source issue * fix: ui issue button group not disabled on page load on entry index page * fix: refactor buttons twig and spinner ui * fix: new translation button visible on new category creation page * fix: feedback changes * fix: error loading order index page when an entry gets deleted * fix: draft action from detail page * fix: new category draft as native flow * update: download tm files action diabled by default * updated: better logging on updating order/files * fix: missing blocks from our draft * fix: target change alert for completed api orders * refactor: removing category repository * update: code refactor * update: revert catagory repository changes * updt: changelog for release-3.0.0-beta.1 * docs(v3.0.0-beta.1): bump version and change log * Update CHANGELOG.md * updt: plugin schema version Co-authored-by: Sumit S <[email protected]>
This reverts commit f47c6d2.
Revert "Craft 4" changes
@@ -109,7 +109,7 @@ public function updateDraft($element, $draft, $translatedContent, $sourceSite, $ | |||
$draft->title = isset($targetData['title']) ? $targetData['title'] : $draft->title; | |||
$draft->slug = isset($targetData['slug']) ? $targetData['slug'] : $draft->slug; | |||
|
|||
$post = Translations::$plugin->elementTranslator->toPostArrayFromTranslationTarget($draft, $sourceSite, $targetSite, $targetData); | |||
$post = Translations::$plugin->elementTranslator->toPostArrayFromTranslationTarget($element, $sourceSite, $targetSite, $targetData); |
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.
@shnsumit @bhupeshappfoster could you please explain the change from $draft
to $element
? This may have unintended consequences.
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.
@sidedwards
this change is because of drafts in target sites never contains localised nested blocks and hence can lead missing blocks from draft so we use source entry to get all content to target in case it is missing from target
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.
@bhupeshappfoster, thanks for the explanation. We will need to do some comprehensive e2e testing to make sure this doesn't break other workflows, but we can proceed with merging for now.
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.
this change from draft to element is not required for the fix give me min i ll commit a revert and then we can proceed with merge and release
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.
@sidedwards
Please review again i think we can proceed with merge now.
Fixed