I'm not sure if this is only an issue with layout builder, but if you have paragraph fields on some content types and not others, the getTitle function in ParagraphBlocksLayoutHelperBase fails because it attempts to get the field delta on the entity before checking if the entity has the paragraph field in question. This patch corrects that and also changes the hookLayoutBuilderChooseBlocksAlter function to check for the FALSE status being returned from the getTitle function, so that it will function the same as the panels version of the function. With those two changes the layout manager works flawlessly with this module now for me.

Comments

jacobbell84 created an issue. See original summary.

douggreen’s picture

Status: Needs review » Needs work

Please retry with the 8.x-2.x branch and re-apply any changes there. Here are a few comments:

  1. +++ b/src/ParagraphBlocksLayoutHelperBase.php
    @@ -82,7 +82,11 @@ class ParagraphBlocksLayoutHelperBase {
    +        if ($title === FALSE) {
    

    This has already been made to the 8.x-2.x branch, but it's a good catch.

  2. +++ b/src/ParagraphBlocksLayoutHelperBase.php
    @@ -105,21 +109,30 @@ class ParagraphBlocksLayoutHelperBase {
    +    if ($plugin_type_id != 'paragraph_field') {
    

    This may still be needed. Please retry on the 8.x-2.x branch.

  3. +++ b/src/ParagraphBlocksLayoutHelperBase.php
    @@ -105,21 +109,30 @@ class ParagraphBlocksLayoutHelperBase {
    +        if ($this->entity->hasField($plugin_field_name))
    

    Why would you ever have the plugin without the field existing on the entity. That seems like a configuration error?

    Also, please use proper coding standards. See https://www.drupal.org/docs/develop/standards

jacobbell84’s picture

Version: 8.x-1.1-beta6 » 8.x-2.1-beta3
StatusFileSize
new17.81 KB
new662 bytes

Hi douggreen,
Thank you for the feedback. #2 doesn't seem to be needed anymore, in my tests it worked fine without any additional changes. I do believe #3 is still needed, though it is partially mitigated by being able to turn off paragraph blocks on specific paragraph fields. Let me give an example of my use case so you can reproduce the issue:

1) Create two node bundles in the system (Test Node A and Test Node B).
2) Create two paragraph entity reference fields (Test Field A and Test Field B).
3) Add Test Field A to Test Node A and Test Field B to Test Node B.
4) Edit the layout on Test Node A and click the "Add Block" link in any section.
5) It will appear as though nothing happened, though there is a generic error in the browser console.
6) Checking the php logs there will be an error: InvalidArgumentException: Field field_test_field_b is unknown. in Drupal\Core\Entity\ContentEntityBase->getTranslatedField() (line 586 of web\core\lib\Drupal\Core\Entity\ContentEntityBase.php).

I believe this is because the block deriver doesn't have an entity context when it's generating the paragraph blocks so even though the field isn't associated to the node it's still showing up in the list as a possibility. Now that it's possible to disable paragraph blocks on specific fields I could turn it off for Test Field B, assuming I don't need paragraph blocks on that, but it still seems like a good idea to check in case somebody forgets to turn off that setting on other fields.

Adding bundles to the module has introduced another similar situation with the same line of code causing the above issue, so I'm attaching it here. Let me know if you'd rather I open a separate issue. The bundle related problem is that I have a paragraph blocks field added to 7 node bundles on the system, all of which use it for their layout builder. Without checking to see if paragraph plugin bundle type matches the current entity bundle type, you end up with every delta being duplicated 7 times on the layout manager choose block screen:

Choose block edit screen

I attached a patch that does fix both of these issues for me, but I feel like there's probably a cleaner way to do this, since the getTitle function is starting to get a little bloated. Maybe a separate service?

jacobbell84’s picture

Status: Needs work » Needs review
douggreen’s picture

+++ b/src/ParagraphBlocksLabeller.php
@@ -160,7 +160,7 @@ class ParagraphBlocksLabeller {
+    if (!$this->entity || !$this->entity->hasField($plugin_field_name) || $this->entity->bundle() != $plugin_field_bundle || $plugin_field_delta >= $this->entity->get($plugin_field_name)->count()) {

The bundle check makes sense. But I don't think we need to check for the field. If you remove the bundle check from this patch, and run your test case, does it still fail?

jacobbell84’s picture

Good point, now that bundles are in play it will solve for both issues. Tested and checking just the bundle resolved both my problems. New patch is attached.

douggreen’s picture

Status: Needs review » Fixed

committed, thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

jacobbell84’s picture