$root_parent not as optional as declared

Created on 10 February 2025, about 2 months ago

Problem/Motivation

I encountered the following error when working with Frontend Editing and All Entity Preview :
TypeError: Drupal\paragraphs_edit\ParagraphLineageRevisioner::shouldCreateNewRevision(): Argument #1 ($entity) must be of type Drupal\Core\Entity\EntityInterface, null given, called in /var/www/drupal/docroot/modules/contrib/paragraphs_edit/src/Form/ParagraphEditForm.php on line 65 in Drupal\paragraphs_edit\ParagraphLineageRevisioner->shouldCreateNewRevision() (line 111 of modules/contrib/paragraphs_edit/src/ParagraphLineageRevisioner.php).

Digging into shows that the called handler: \Drupal\paragraphs_edit\ParagraphFormHelperTrait::buildForm() accepts an additional argument: ?EntityInterface $root_parent = NULL
While it is declared as optional, it isn't really down the line.

And while the cause for the missing argument is a bug 🐛 PreviewFormService::alterForm not passing on generic arguments Active in All Entity Preview it made me wonder how this optional non-optional argument should be handled.

Proposed resolution

A straight forward approach would be to adjust \Drupal\paragraphs_edit\Form\ParagraphEditForm::save() like so:

if ($this->rootParent && $this->lineageRevisioner()->shouldCreateNewRevision($this->rootParent)) {

However, in case of a bug 🐛 PreviewFormService::alterForm not passing on generic arguments Active like in All Entity Preview locating the issue would be waaay harder due the missing explicit error.

A middle way could be to log an error, if it is indeed an error, about the missing argument but still trigger a save, using the one not causing any data loss (so creating new revisions I guess).

Ultimately I'm not familiar enough with the logic or data architecture to make an informed proposal for or against an explicit error here.

Remaining tasks

User interface changes

None

API changes

Kinda, because it might not break anymore in the expected way :P

Data model changes

None

🐛 Bug report
Status

Active

Version

3.0

Component

Code

Created by

🇨🇭Switzerland das-peter

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

Merge Requests

Comments & Activities

  • Issue created by @das-peter
  • Pipeline finished with Success
    about 2 months ago
    Total: 155s
    #427377
  • 🇮🇳India kulpratap2002

    I’ve addressed the TypeError in ParagraphEditForm::save() by adding a check for $this->rootParent before calling shouldCreateNewRevision().

    Since the missing rootParent seems to be caused by a bug in the All Entity Preview module, I opted for a middle-ground approach:

    If rootParent is missing, a warning is logged to help with debugging.
    The form still triggers a save—defaulting to creating a new revision— to prevent potential data loss.
    I didn’t use a hard error (like throwing an exception) to avoid breaking content workflows. Let me know if you'd prefer stricter error handling or further adjustments!

  • Pipeline finished with Success
    6 days ago
    Total: 464s
    #462181
  • Pipeline finished with Success
    6 days ago
    Total: 157s
    #462217
  • 🇮🇳India debrup

    The changes look good to me. Installed the module and did some manual testing. Working as expected with no errors.

    @bbrala Please check the recent changes and merge.

  • 🇮🇳India abhishek@kumar

    Strict Parameter Enforcement

    // In ParagraphEditForm.php
    public function buildForm(array $form, FormStateInterface $form_state, ?EntityInterface $root_parent = NULL) {
      if (!$root_parent) {
        throw new \InvalidArgumentException('The root parent entity must be provided.');
      }
      // Rest of the method
    }
    
Production build 0.71.5 2024