Skip to content
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

[4.x]: Matrix blocks not propagated properly when blocks contain link plugin field #12858

Closed
tommysvr opened this issue Mar 10, 2023 · 6 comments

Comments

@tommysvr
Copy link

What happened?

Description

Seeing a strange issue where Matrix blocks are missing after propagating to another entry. The strange part is that I can only reproduce when the entry contains an entry Typed Link Field, LinkIt or Hyper field. I’m not able to reproduce when the entry only contains blocks with native fields.

Seems related to sebastian-lenz/craft-linkfield#233

Steps to reproduce

  1. Have a multisite install with any of Typed Link Field, LinkIt or Hyper installed
  2. Add a Section with Propagation Method 'Let each entry choose which sites it should be saved to'
  3. Add a Matrix field with a text field block and a block for any other link plugin fields. Propagation Method is 'Save blocks to all sites the owner element is saved in'
  4. Create a new entry with a text field block and an empty link field
  5. Propagate the entry to another site, the link field will be marked as changed
  6. View the entry on the other site, only the link field block will have propagated

Expected behavior

All blocks will be propagated

Actual behavior

Only the link fields are propagated

screenshot_2023_03_10._11.34.mp4

Craft CMS version

4.4.1

PHP version

No response

Operating system and version

No response

Database type and version

No response

Image driver and version

No response

Installed plugins and versions

@engram-design
Copy link
Contributor

engram-design commented Mar 10, 2023

I'm investigating this issue as well. I've shortened the reproduction slightly:

  1. Create a new entry in Site A.
  2. Populate the title, then add a Text and Link Matrix block. Populate those fields with a value
  3. Save the entry (in Site A)
  4. Add the entry to Site B, and notice the change indicators. No need to re-save the entry in Site A
  5. Switch to Site B and notice the blocks being incorrect

Here's a video
https://share.cleanshot.com/rfMW6Hzx

The only thing that looks fishy to me is the request payload at step 4 as soon as you pick "Site B" from the status (the initial propagation to the other site):

title: 1
newWindow: 
elementId: 157
siteId: 1
slug: 1-2
enabled: 1
enabledForSite[1]: 1
enabledForSite[2]: 1
notes: 
fields[matrix][blocks][161][fields][linkField]: 
fields[matrix][blocks][161][fields][linkField][0][type]: verbb\hyper\links\Url
fields[matrix][blocks][161][fields][linkField][0][handle]: default-verbb-hyper-links-url
fields[matrix][blocks][161][fields][linkField][0][newWindow]: 
fields[matrix][blocks][161][fields][linkField][0][linkText]: 3
fields[matrix][blocks][161][fields][linkField][0][id]: tcpobwynfv
fields[matrix][blocks][161][fields][linkField][0][linkValue]: 
fields[matrix][blocks][161][fields][linkField][0][linkText]: 3
modifiedDeltaNames[]: fields[matrix][blocks][161][fields][linkField]
visibleLayoutElements[e3ef0647-3324-4acb-bce8-47dd23c8ac0c][]: bdaf3f66-ee87-4a6e-a9f8-632ad6b5a734
visibleLayoutElements[e3ef0647-3324-4acb-bce8-47dd23c8ac0c][]: 6487f158-c70e-4f20-898b-35bb5c050693
provisional: 1
selectedTab: tab--content
action: elements/save-draft

So what's interesting here is that linkField content is included here, but the text content is missing - hence the block isn't created. Or, more to the point maybe it shouldn't be included in this propagation step.

I'm just trying to figure out what's the correct thing here. Firstly, it seems incorrect that the field is being marked as changed on "Site A" when I've done nothing to change that value. I've only propagated a new entry on "Site B".

Next, Hyper (and other fields) seem to be doing the correct thing here by copying the content from "Site A" to the new entry in "Site B", and it's everything else that seems to be incorrect. Is that right? Or should a new entry in another site have empty content?

@engram-design
Copy link
Contributor

engram-design commented Mar 10, 2023

This can also be confirmed by simplifying the Hyper field to remove all Vue components and reduce it to a single text field for the Link Text. This at least helps isolate things.

image

Again, in Step 4 where I've just picked the other site to add the entry to, you can see the changed content is registered for the Matix field overall, the Hyper field, but not the Text field. Technically, Hyper is in the wrong here, as the content hasn't changed, so I (and the other plugins) are possibly doing something wrong.

After more testing, if I change the value of the Hyper field to be a simple value, and not a LinkCollection the issue goes away. This makes sense if other plugins are having this issue, as all of them use objects and not simple values.

@engram-design
Copy link
Contributor

And just to simplify it even further (or maybe I'm going down the wrong path here), is that fields are incorrectly marked as being modified on "Site A" when all I've done is propagate to "Site B" - even when outside of a Matrix field.

image

After more testing, if I change the value of the Hyper field to be a simple value, and not a LinkCollection the issue goes away. This makes sense if other plugins are having this issue, as all of them use objects and not simple values.

I did pick up on the below check which thought might be the key to this but omitting it doesn't do anything.

cms/src/services/Elements.php

Lines 1488 to 1493 in 73771c5

// Clone any field values that are objects
foreach ($mainClone->getFieldValues() as $handle => $value) {
if (is_object($value) && (!class_exists(UnitEnum::class) || !$value instanceof UnitEnum)) {
$mainClone->setFieldValue($handle, clone $value);
}
}

@brandonkelly
Copy link
Member

The culprit here was these lines:

if (empty($element->id) || $element->isFieldEmpty($this->_field->handle)) {
$view->setInitialDeltaValue($this->_field->handle, null);
}

Which tell the element editor to ignore whatever the input’s actual initial post value is, and pretend it was null, if the field type’s isValueEmpty() method returned true.

That logic was originally added in d13a51f, to fix a bug where Dropdown fields that had no value weren’t getting their default option to be selected on save, despite it looking like it was, because the input value on save matched what it was on initial page load (#5632).

But that’s actually not needed anymore as of 4.4, thanks to e2eb651, which adds a blank option to Dropdown fields that don’t have a value yet (#12235).

So those lines have been removed for the next release, and now Hyper (and theoretically similar fields) now behaves as expected – only being seen as modified when the post data actually changes.

@engram-design
Copy link
Contributor

Thanks for the investigation @brandonkelly !

@brandonkelly
Copy link
Member

4.4.2 is out with that change!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants