Problem/Motivation

This is a related issue to https://www.drupal.org/project/diff_plus/issues/3485245

There's a bit of an odd design pattern that I've not encountered before with the diff module configuration. Essentially, when a third party module adds a new diff layout plugin type, unless the third party module also adjusts diff.settings:layout_plugins on install, the next time a site owner visits the diff general settings form they'll encounter a pile of warnings:

A collection of php warnings that are explained in more detail following this image

Steps to reproduce

  1. Install the diff module.
  2. Install the diff_plus module.
  3. Visit the diff general settings page. Observe warnings.
  4. Save the diff general settings page. Warnings are now gone (because the diff settings were updated).

Proposed resolution

Update the diff module settings form to be more lenient and assume that new plugins are disabled by default (and may not yet exist in active configuration).

If the diff module encounters any unknown plugin types, it should assume that they are disabled and that their weights are greater than the highest known plugin weight.

Remaining tasks

Fork, MR, review, merge, profit.

User interface changes

None

API changes

None

Data model changes

None

CommentFileSizeAuthor
diff-warnings.png465.24 KBluke.leber

Issue fork diff-3515557

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

luke.leber created an issue. See original summary.

luke.leber’s picture

luke.leber
Primary language English
Location Pennsylvania
commented
Issue tags: +Atlanta2025
acbramley’s picture

acbramley commented
Category: Support request » Bug report

Definitely should be doing the second proposed solution, there should be defense in that buildForm method for when $config->get('general_settings.layout_plugins')[$id] returns NULL

luke.leber’s picture

luke.leber
Primary language English
Location Pennsylvania
commented
Title: A crossroads: how to resolve warnings when a module adds new diff plugins? » Harden general settings form against newly added layout plugins
Issue summary: View changes

Update title + IS.

Thanks Adam.

luke.leber opened merge request !158

luke.leber’s picture

luke.leber
Primary language English
Location Pennsylvania
commented
Status: Active » Needs work

NW for phpstan fails that I don't quite understand.

 ------ ----------------------------------------------------------------------- 
  Line   src/Plugin/diff/Field/EntityReferenceFieldBuilder.php                  
 ------ ----------------------------------------------------------------------- 
  36     Access to an undefined property                                        
         Drupal\Core\Field\FieldItemInterface::$entity.                         
         🪪  property.notFound                                                  
         💡  Learn more: https://phpstan.org/blog/solving-phpstan-access-to-und  
         efined-property                                                        
 ------ ----------------------------------------------------------------------- 
 ------ ------------------------------------------------------------- 
  Line   src/Plugin/diff/Field/ListFieldBuilder.php                   
 ------ ------------------------------------------------------------- 
  37     Call to an undefined method                                  
         Drupal\Core\Field\FieldItemInterface::getPossibleOptions().  
         🪪  method.notFound                                          
 ------ -------------------------------------------------------------

Seems unrelated to this MR; possibly something to fix in HEAD?

acbramley’s picture

acbramley commented
Status: Needs work » Fixed

Fixed!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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