Problem/Motivation
On a layout builder page, creating and adding a block that references a paragraph entity leads to the display of "Orphaned 'name of the paragraph':" (which is the label of the paragraph entity) instead of the paragraph entity's preview. This happens only with those paragraphs that are displayed via a custom formatter (in this case: Drupal\slick_paragraphs\Plugin\Field\FieldFormatter\SlickParagraphsVanillaFormatter) that extends the Drupal\slick\Plugin\Field\FieldFormatter\SlickEntityFormatterBase class which extends the Drupal\blazy\Dejavu\BlazyEntityBase
The piece of code that does this:
if ($entity->id()) {
$this->buildElement($build, $entity, $langcode);
// Add the entity to cache dependencies so to clear when it is updated.
$this->formatter()->getRenderer()->addCacheableDependency($build['items'][$delta], $entity);
}
else {
$this->referencedEntities = NULL;
// This is an "auto_create" item.
$build['items'][$delta] = ['#markup' => $entity->label()];
}
is inside the `buildElements` method of Drupal\blazy\Dejavu\BlazyEntityBase
Steps to reproduce
- Create a paragraph type: Slide
- Add fields to paragraph: Title (text), Media (Image), Description (text - formatted, long)
- Create a custom block type: Slider
- Add field to block type: Slide (Paragraph reference: Slide)
- Manage display of block type: Slide (Formatter: Slick Paragraphs Vanilla)
- On a page with layout builder, add One column section
- Click Add component in the section
- Create a custom block - Slider and add it on the page
Expected behaviour: Rendered preview of the slider.
Actual behaviour: "Orphaned 'name of the paragraph':"
Proposed resolution
This behaviour is observed only when the block is newly created and upon saving the page layout and revisiting the block it is rendered correctly with its preview. So, as seen in the above code, this is because only the label is being displayed for an "auto_create" item and commenting the condition outputs the expected behaviour. It would be better to know more on why this condition is in place and what should be done in this case.
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | 3214001-3.patch | 1.06 KB | vinay15 |
Comments
Comment #2
gausarts commentedThank you, and I appreciated your kind detailed info.
> 5. Manage display of block type: Slide (Formatter: Slick Paragraphs Vanilla)
There might be a few formatters in there. Do they behave similarly, or just
Slick Paragraphs Vanillais the bad boy?If they behave similarly, nothing we can't do. If the only bad boy, it deserves more love.
> commenting the condition outputs the expected behaviour.
Good to know, and needs a follow up, might be a clue to solutions.
AFAIK, normally
$entity->id()only exists once saved into database, that is why it behaves correctly as you described. It is kind of a guard to prevent potential errors due to the entity not being saved, yet.I said normally, because there is a possibility to have non-incremented ID such as UUID. At least in my JAVA app with sqLite. I am not aware if Paragraphs uses UUID as entity ID or of any modules at Drupal in general which use UUID to replace regular database row ID. If anyone knows, feel free to correct me, and point me to such module. With UUID in place, it is possible to have such previews before saving with the current check, it will be a matter of more fine-grained checks to avoid warnings and errors.
The only reason that ID check exists is so it doesn't introduce more errors due potential unsaved values.
Shortly, if without ID doesn't introduce errors, we should get rid of that useless check.
Patches are welcome, and of course thorough checks with all those options should be performed to ensure no regressions.
Comment #3
vinay15I have removed the check and tested that no warnings or errors are thrown. Also,
$entity->uuid()is available for all the entities even before these entities are stored in the database and an$entity->id()is associated to them. So adding the UUID check would also act as a guard here, but since there were no errors or warnings, I have removed the condition completely.Comment #4
gausarts commented> but since there were no errors or warnings, I have removed the condition completely.
Although I have a doubt, mostly because I am lacking of dev setups currently, but I am fine committing this. We can always revert if any uncovered issue later :)
If anyone find an obvious issue we missed, kindly report. Thank you.
Please allow some delay to get back on this.
Comment #6
gausarts commentedWe'll revert if any uncovered issue. Hopefully not.
Committed. Thank you for contribution.