Use groups in Pattern presets library

Hi @juc1, thanks for the work, can you:

• fix pipelines errors (mostly phpcs)?
• move group logic to its own protected property in PresetLibraryPanel?

The prefill of group value with the SDC group is unexpected but looks like a good idea :) Maybe we can do the same with other sources ?

• if Drupal block plugin, the block category
• for other slot sources, just the source label

Then, users will be able to edit the group in admin UI anyway.

Hi @juc1,

Thanks for your proposal and proposing cool additional mechanisms like the prefill of the value. It is very lovely.

Review

However, I am a bit concerned this little feature became a +233 LOC addition with:

• a ComponentPluginManager dependency in ApiController
• some ComponentSource data structure logic in ApiController::saveAsPreset()
• a ComponentPluginManager dependency in PresetLibraryPanel

We need to keep parts of the architecture cleanly separated. ApiController and PresetLibraryPanel must not be aware of ComponentPluginManager and ComponentPluginManager, because there must be the Source API as a proxy in-between.

Proposal

What do you think about proposing to UI Patterns' team to add something like:

So:

• your PresetLibraryPanel::getComponentPresetGroup() can become SourceComponentSource::getGroup()
• your PresetLibraryPanel::getBlockPresetGroup() can become BlockSource::getGroup()
• your PresetLibraryPanel::getOtherSourcePresetGroup() can become SourcePluginBase::getGroup()

That's all for the scope of this issue. After that, we can create a follow-up ticket to use the new methods to get rid of BlockLibrarySourceHelper because most of the logic here will reside in Source plugins themselves.

Let's discuss :)

Hi @juc1,

I will push an additional commit to your MR with the move of the logic.

We have a look together. If we like that, we create an UIP2 issue with the proposal.

Back to Juc1 :)

A little tip: you can move the initialization logic from ApiController to PatternPreset entity by making $group nullable:

-  protected string $group = '';
+  protected ?string $group = NULL;

and extending ::getGroup() method:

   public function getGroup(): string {
    if (is_null($this->group) && isset($this->sources['source_id'])) {
      $configuration = [
        'settings' => $this->sources['source'] ?? [],
      ];
      /** @var \Drupal\ui_patterns\SourceInterface $source */
      $source = $this->sourcePluginManager()->createInstance($this->sources['source_id'], $configuration);
      $this->group = $source->getGroup();
    }
     return $this->group;
  }

We will also need a unit test for this

Thanks, i will have a look and add a commit with the documentation update.

Thanks for the incredible work, with a lot of clear comments and a kenrel test.

I will do 4 little changes before RTBC:

• a little documentation update
• run php-cs-fixer (not mandatory but some team members run this linter locally, so it will make their life easier)
• an implementation of ::getGroup() in \Drupal\display_builder\Plugin\UiPatterns\Source\ComponentSource (this class is temporary, it will be move to UI Pattenrs later, but it will already be useful to get the implementation)
• a logic change: once saved in the config entity, the group must not be automatically changed, even if the ::getGroup() logic change. ::getGroup() is used only when creating the pattern preset.

We did it!