if ($field->getFieldDefinition()->getTargetEntityTypeId() == "paragraph") { compare to FieldConfigBase:;getTargetEntityTypeId: return $this->entity_type; meaning if you have an entity revisions reference field it will always get duplicated even if the target is not a paragraph.
The same method WSODs hard if the entity referenced doesn't exist.
Change to getSetting('target_type') and check whether the referenced entity exists.
Do it.
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | 3089724_cleanup.txt | 1.56 KB | ghost of drupal past |
| #9 | 3089724_9.patch | 1.46 KB | ghost of drupal past |
Comments
Comment #2
ghost of drupal pastI also fixed a problem where duplicating a broken reference wsods.
Comment #3
ghost of drupal pastComment #4
ghost of drupal pastI believe this is much nicer. (It doesn't check for a revisions reference but IMO that's irrelevant.)
Comment #5
berdirI'm not sure about that being better. The behaviour of paragraphs not being reusable is tied to ERR fields. We currently actually don't allow creating ER fields with paragraphs as target to avoid misconfiguration, but did in fact recently have a case for doing so. Not on a paragraph entity in that case, but still.
Comment #6
berdirIt also wouldn't be in sync with \Drupal\paragraphs\EventSubscriber\ReplicateFieldSubscriber anymore.
Comment #7
ghost of drupal pastSure. I just liked the interface better, this class doesn't have an interface but if this is what's needed, I can. The reason I thought it's nicer not because of that detail but because it drops the definition and also there are no strings, and also no ->entity: retrieval is via referencedEntities() and setting happens via the ItemList::offsetSet => ItemList::set => EntityReferenceRevisionsItem::setValue call chain so no need for ->entity.
Comment #8
berdirIt might look nicer, but currently the referencedEntities() implementation is pretty bad, it always calls loadRevision() and there's no static cache for that yet. So I'd prefer to work with $item->entity, sorry :)
Not sure about a test, we have \Drupal\paragraphs\Tests\Experimental\ParagraphsExperimentalDuplicateFeatureTest::testDuplicateButtonWithNesting(), we could add a second ERR field that points to nodes or so, add a paragraph with two references, delete one to test the extra if check and then load the node after duplicating to make sure that the reference points to the same ID?
Comment #9
ghost of drupal past> It might look nicer, but currently the referencedEntities() implementation is pretty bad, it always calls loadRevision() and there's no static cache for that yet
...
which is not only a performance issue but also leads to data loss if the subparagraph has changes. #3098924: referencedEntities() causes data loss
So yes
$item->entityit is.Comment #10
berdirCommitted.
Comment #12
ghost of drupal pastWe had a problem where blocks were duplicated like no tomorrow due to this bug. Tonight I cleaned it up. The script is worthy of posting here.