Create a trait and base class to implement \Drupal\Component\Plugin\ConfigurableInterface

Created on 14 February 2017, almost 8 years ago
Updated 26 August 2024, 4 months ago

Problem/Motivation

#2851635: DefaultSingleLazyPluginCollection retains stale instance IDs shows that base plugin classes should all merge default configuration in the same way. We should make it easy to implement \Drupal\Component\Plugin\ConfigurableInterface.

Proposed resolution

Add new trait and base class and use it where appropriate in core.

Additionally, since the use of this interface requires some work to be done in the constructor, add a base class which does that work in the constructor. Not all plugins will be able to use the base class, as they may need to extend a different class, but it will be useful for those that can. Those that can cannot use the base class can use the trait and reimplement the constructor logic in their own constructor.

Specifically:

Add

  • Drupal\<del>Component</del>Core\Plugin\ConfigurableTrait
  • Drupal\Core\Plugin\ConfigurablePluginBase

Use the new base class in the following classes:

  • Drupal\Core\Display\VariantBase
  • Drupal\Core\Entity\EntityReferenceSelection
  • Drupal\Core\Layout\LayoutDefault
  • Drupal\image\src\ImageEffectBase
  • Drupal\workflows\src\Plugin\WorkflowTypeBase

Use the new trait in the following classes:

  • Drupal\Core\Action\ConfigurableActionBase
  • Drupal\search\src\Plugin\ConfigurableSearchPluginBase

Adjust related tests as needed, and add Drupal/Tests/<del>Component</del>/Plugin/ConfigurableTraitTest.

Remaining tasks

User interface changes

None

API changes

New trait

Data model changes

None

Release notes snippet

ConfigurablePluginBase and ConfigurableTrait have been added to help plugins implement ConfigurableInterface in a consistent way. The following base plugin classes have been converted to use the new boilerplate.

ConfigurableActionBase
VariantBase
SelectionPluginBase
LayoutDefault
ImageEffectBase
ConfigurableSearchPluginBase
WorkflowTypeBase

📌 Task
Status

RTBC

Version

11.0 🔥

Component
Plugin 

Last updated 9 days ago

Created by

🇬🇧United Kingdom alexpott 🇪🇺🌍

Live updates comments and jobs are added and updated live.
  • Contributed project blocker

    It denotes an issue that prevents porting of a contributed project to the stable version of Drupal due to missing APIs, regressions, and so on.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • 🇮🇹Italy gambry Milan

    Swapping the Drupal Global Contribution Weekend 2023 tag with the correct one.

  • First commit to issue fork.
  • 🇮🇳India bhanu951

    Add missing return statement and fix CCF.
    Rebased to latest head.

    # 132 📌 Create a trait and base class to implement \Drupal\Component\Plugin\ConfigurableInterface Needs review and # 123 📌 Create a trait and base class to implement \Drupal\Component\Plugin\ConfigurableInterface Needs review still need discussion.

  • Merge request !8689Resolve #2852463 "Configurable trait and base" → (Open) created by mikelutz
  • 🇺🇸United States mikelutz Michigan, USA

    mikelutz changed the visibility of the branch 11.x to hidden.

  • 🇺🇸United States mikelutz Michigan, USA

    mikelutz changed the visibility of the branch 11.x to hidden.

  • 🇺🇸United States mikelutz Michigan, USA

    mikelutz changed the visibility of the branch 2852463-configurable-plugin-trait-10.1 to hidden.

  • 🇺🇸United States mikelutz Michigan, USA

    mikelutz changed the visibility of the branch 9.2.x to hidden.

  • 🇺🇸United States mikelutz Michigan, USA

    mikelutz changed the visibility of the branch 2852463-create-a-trait to hidden.

  • Pipeline finished with Failed
    6 months ago
    Total: 180s
    #218024
  • Pipeline finished with Failed
    6 months ago
    Total: 180s
    #218040
  • Pipeline finished with Failed
    6 months ago
    Total: 182s
    #218098
  • Pipeline finished with Canceled
    6 months ago
    Total: 488s
    #218101
  • Pipeline finished with Failed
    6 months ago
    Total: 517s
    #218103
  • Pipeline finished with Failed
    6 months ago
    Total: 579s
    #218108
  • Pipeline finished with Failed
    6 months ago
    Total: 596s
    #218116
  • Pipeline finished with Failed
    6 months ago
    Total: 683s
    #218165
  • Pipeline finished with Success
    6 months ago
    Total: 539s
    #218887
  • 🇺🇸United States mikelutz Michigan, USA
  • Pipeline finished with Success
    6 months ago
    Total: 539s
    #219035
  • Status changed to Needs review 6 months ago
  • 🇺🇸United States mikelutz Michigan, USA

    Alright, I think I've resurrected this from the ashes. New merge request on 11.x, fixed issues in #123and #132

    123.1 Mention the interface instead of the trait in ConfigurablePluginBase.

    I mention both, since the base class uses the trait anyway.

    123.2 In ConfigurableTrait, document that the $configuration property is also defined \Drupal\Component\Plugin\PluginBase.

    done

    123.3 Decide on if ConfigurableTraitTest needs adjustments.

    It did, though opposite from the previous suggestions of moving all tests to getMockForTrait, as getMockForTrait is getting deprecated in phpunit, so I converted all tests to use the same concrete test class that uses the trait.

    123.4 Make sure that the changes in base classes for certain plugin types are mentioned in the release notes.

    I added the list to the release notes snippet and the CR

    123.5 (optional) Check if setConfiguration() still needs to be overridden in WorkflowTypeBase.

    It's an issue of overriding it and keeping tests, or resetting it to the standard and adjusting all the tests to handle the reordered returned array. I also removed use of the trait entirely from ConditionPluginBase for the same reasons, updating it to use mergeDeepArray broke a few tests and will require adjusting the order of items in umami config to pass tests (as they would be exported in a different order, causing a test fail) It's better to explore converting both of those pluginbases to use mergeDeepArray in a followup, if at all, where we can review the test changes and potential BC issues more thoroughly in isolation from the trait and base class work being provided here.

    123.6 Rebase the merge request.

    done.

    132 > implement \Drupal\Component\Plugin\ConfigurableTrait directly

    Is that the right wording? Does one 'implement a trait'? I'd have said 'use' or 'import'.

    I agree, switched the wording to use `use`.

    In addition, I added SelectionPluginBase to the plugin bases that we convert (it was added after the original MR was created here), and removed ConditionPluginBase for reasons described above.

    I moved ConfigurableTrait from Component/Plugin to Core/Plugin. In the original patch, we didn't notice that having the trait in Component meant we added a undocumented dependency on Component/Utility to Component/Plugin, and we shouldn't do that. The interface can live in the Component, the trait can live in Core next to the base class (which was already there because that needed to extend Core's PluginBase and not Component's. Despite the fact that the interface lives in Component, it makes more sense to have the implementations live in Core.

    I updated the Change Record (and unpublished it, as it was never unpublished after we reverted years ago.

    I fixed a few coding standards/phpstan/phpunit10 issues that have changed things over the years.

    I think this should be committable now, or close to it hopefully.

  • First commit to issue fork.
  • Pipeline finished with Success
    4 months ago
    Total: 608s
    #261557
  • Pipeline finished with Failed
    4 months ago
    Total: 237s
    #261581
  • Status changed to RTBC 4 months ago
  • 🇺🇸United States smustgrave

    Opened 📌 Determine if we can remove logger from ImageEffectBase Active to research if we can remove logger.

    All I did was rebase (200+ commits back) and added some typehints to the test classes.

    Reviewed the rest of the changes and looks cleaner. Didn't see anything off so lets see if we can land this old one.

  • Pipeline finished with Success
    4 months ago
    Total: 509s
    #261586
  • 🇳🇿New Zealand quietone

    I read the IS, the comments and the MR. There are suggestions in the MR for the comments and they do need someone working on this issue to review, so back to NW for that.

    And a follow up is needed for this part of #145

    It's better to explore converting both of those pluginbases to use mergeDeepArray in a followup, if at all, where we can review the test changes and potential BC issues more thoroughly in isolation from the trait and base class work being provided here.

  • Pipeline finished with Failed
    about 2 months ago
    Total: 519s
    #329211
Production build 0.71.5 2024