Add proper check if there is no paragraph created yet in the entity

Created on 30 May 2023, over 1 year ago
Updated 6 June 2023, over 1 year ago

Problem/Motivation

First of all, thank you very much for creating this module. It has been one of the most useful modules we have used with layout builder.

We have been trying to solve the problems / feature requests of #3088576: Add the possibility to create paragraph directly from Layout Builder and Ability to edit paragraph content directly from Layout Builder UI Active by writing a new module: https://www.drupal.org/project/layout_builder_paragraphs . The module is still far from providing perfect authoring nor dev nor site builder experience, but we have used it for few live projects and it has definitely helped our clients to use layout builder with paragraphs smoothly.

As we only create the paragraph in the layout builder, the paragraph does not exist yet when php executes ParagraphBlock::buildConfigurationForm(). This resulted in error in the following lines:

      $paragraph = $this->getLatestParagraph($entity);
      if (!$paragraph->hasAdminTitle()) {

Because $paragraph is null in our case. So I add a small value check that should not disrupt how this module works. Iirc this issue only came about in 3.x.

I'm hesitating to put the category as feature request or bug. Paragraph blocks work OK independently. But when the form needs to be rendered programmatically from other place, it does not provide enough error handling.

Steps to reproduce

1. Install D9 / D10.
2. Install layout_builder_paragraphs:1.0.0 (which requires paragraph_blocks:3.1.3, higher version causes other errors, which I'll submit separate issues).
3. Configure the modules following layout_builder_paragraphs readme.
4. Create new node with layout builder, add paragraph in layout builder, and the error is there as shown in the screenshot.

Proposed resolution

Just add value check and assign uuid as the admin title.

Remaining tasks

Review and approve the patch.

User interface changes

None, as this failure mode only happens when using this module with layout_builder_paragraphs.

API changes

None.

Data model changes

None.

Feature request
Status

Fixed

Version

3.0

Component

Code

Created by

🇸🇬Singapore loziju

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @loziju
  • @loziju opened merge request.
  • Status changed to Needs review over 1 year ago
  • 🇸🇬Singapore loziju

    forgot to change the status!

  • Assigned to basvredeling
  • Status changed to Needs work over 1 year ago
  • 🇳🇱Netherlands basvredeling Amsterdam

    @loziju I did a quick review of the patch. Why do we need a value in the admin title? And why should it be a uuid?
    I think I'd prefer it to be an empty string.

    Also have you tested the latest 3.1.5 version?

  • 🇸🇬Singapore loziju

    @basvredeling thanks for looking into this!

    It's not really about admin title (paragraph), but about paragraph_field block label (LB block field, see the screenshot ). Admin title can be blank, as you correctly identified (thus your code tries to get the summary when it's blank). But block label must be populated, even when it's suppressed.

    Since that part of the code is trying to obtain the admin title, and then assign it to the label field, I chose to update the code at that portion. After all, layout_builder_paragraphs will need to create the paragraph_field block first before creating the paragraph and therefore there is nothing to be used as label. So might as well using uuid as it's guaranteed to be unique. To be precise, the actual admin title for the paragraph can be blank or non-blank, just that the paragraph is not created yet at the point of creation of paragraph_field block form, thus we need to use some other value for the block label. And more importantly, beyond layout_builder_paragraphs, other new contrib / custom modules can always utilise paragraph_field block form without existing paragraph, so it's important to have proper error handling here.

    Furthermore, the purpose of block label is to show the text when "Show content preview" checkbox is unchecked in LB. The admin user of layout_builder_paragraphs can always update the label after the paragraph is inserted into LB. But upon creating the paragraph / paragraph_field block itself, the label is not important.

    And finally, for typical use case of paragraph_blocks module, assigning uuid to block label will never happen. This only happens when paragraph is not created while paragraph_field block form is already built.

    As for 3.1.5, it gave error with layout_builder_paragraphs, but I didn't have time to look into it yet. I reckon it's due to the new arguments added for the services, which is introduced in 3.1.4. What is inside 3.1.5 that you'd like me to test?

  • Status changed to Fixed over 1 year ago
  • 🇳🇱Netherlands basvredeling Amsterdam

    Thanks for the clarification. It's a small patch, without any major consequences so I've merged it into 3.x-dev.
    I'll try to add it to a release as soon as I've got some more features/fixes to release.

    Thanks for your contribution. I'm eager to see more of that layout_builder_paragraphs module you're working on. Do keep me in the loop. It sounds like it could also be a submodule for paragraph_blocks.

  • Status changed to Fixed over 1 year ago
  • 🇳🇱Netherlands basvredeling Amsterdam

    Part of the 3.1.6 and 4.0.0-beta1 releases.

Production build 0.71.5 2024