Allow other modules to disable the IPE based on custom logic

So I spent some time on this today to see if I could abstract out a plugin type for this work. The patch is a little rough still, but should communicate the idea. I'll put the workbench_moderation plugin below as an example.

Eclipse

/**
 * @file
 * Contains \Drupal\workbench_moderation\Plugin\IPEAccess\WorkbenchModerationIPEAccess.php
 */

namespace Drupal\workbench_moderation\Plugin\IPEAccess;


use Drupal\Core\Plugin\ContainerFactoryPluginInterface;
use Drupal\panels\Plugin\DisplayVariant\PanelsDisplayVariant;
use Drupal\panels_ipe\Annotation\IPEAccess;
use Drupal\panels_ipe\Plugin\IPEAccessBase;
use Drupal\workbench_moderation\ModerationInformationInterface;
use Symfony\Component\DependencyInjection\ContainerInterface;

/**
 * @IPEAccess(
 *   id = "workbench_moderation_ipe",
 *   label = @Translation("Workbench moderation")
 * )
 */
class WorkbenchModerationIPEAccess extends IPEAccessBase implements ContainerFactoryPluginInterface {

  /**
   * The moderation information service.
   *
   * @var \Drupal\workbench_moderation\ModerationInformationInterface
   */
  protected $information;

  /**
   * WorkbenchModerationIPEAccess constructor.
   *
   * @param array $configuration
   * @param string $plugin_id
   * @param mixed $plugin_definition
   * @param \Drupal\workbench_moderation\ModerationInformationInterface $information
   */
  public function __construct(array $configuration, $plugin_id, $plugin_definition, ModerationInformationInterface $information) {
    parent::__construct($configuration, $plugin_id, $plugin_definition);
    $this->information = $information;
  }

  /**
   * {@inheritdoc}
   */
  public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) {
    return new static($configuration, $plugin_id, $plugin_definition, $container->get('workbench_moderation.moderation_information'));
  }

  /**
   * {@inheritdoc}
   */
  public function applies(PanelsDisplayVariant $display) {
    if (!empty($display->getContexts()['@panelizer.entity_context:entity']) && $display->getContexts()['@panelizer.entity_context:entity']->hasContextValue()) {
      $entity = $display->getContexts()['@panelizer.entity_context:entity']->getContextValue();
      return $this->information->isModeratableEntity($entity);
    }
    return FALSE;
  }

  /**
   * {@inheritdoc}
   */
  public function access(PanelsDisplayVariant $display) {
    $entity = $display->getContexts()['@panelizer.entity_context:entity']->getContextValue();
    return $this->information->isLatestRevision($entity) && !$this->information->isLiveRevision($entity);
  }

}

Yeah, but I could imagine a UI that tells you which plugins will apply. That could be really nice for having insight into what's going on and who's making the decisions about when IPE displays.

Eclipse

+1 to making it configurable for plugins. It can be a bit daunting if you don't know what plugins are working on what. At a minimum it'd be great to have a page that shows all the plugins that apply.

So this class is basically for IPE... but I feel like we should abstract this access plugin pipeline a bit more. Talked on IRC about any display renderer being able to use similar plugins, and I agree.

As far as the access plugins go, I'm not sure there is a need to take it further out than panels at the moment. Maybe this is one of those components that panels manages?

well, I think we need to introduce the idea of a frontend renderer. And allow the access check on those. It's a different classification of thing, and administratively, on the backend, I don't really care about wbm's opinion when we're on a panels (or whatever) admin page. But when we're collaborating on rendering something, if we are using a frontend renderer, we should ask other module's opinions.

Eclipse

At the risk of picking an old fight with EclipseGc :-), why a plugin? This seems like a simple voter pattern of tagged services. What's the value-add of plugins here over "a bunch o' objects that return yep/nope"? This doesn't feel like a user-configurable problem space to me...

It's not a user configurable problem space per say. I've been arguing with myself on this. Beyond the ability to store metadata that can be used in an information UI, I can see an argument for these being a tagged service. Frankly though, I don't think there's a significant benefit either way, and Plugins are frankly a little more ubiquitous. I could be convinced either way.

Eclipse

Looking at this patch, I also feel that tagged services is a more appropriate fit for the problem, if only because they are simpler than plugins and have less overhead. I see no reason to needlessly complicate this fix. Plugins are maybe more flexible in a general sense, but how much flexibility is really needed here? And +1 what @dsnopek said about there being no need for multiple instances of these classes.

In this case, I favor simplicity over flexibility.

Yup, I agree with all of that, however, we'll have to invent something if we want to show which services apply to a given node.

I don't think we have an information UI for anywhere else that tagged services are used, and it's fine. Not everything needs to have a complex introspection UI built for it. (In fact, many things probably shouldn't.) Worst case, there's always drupal console debug tools, which are already quite handy.

I wasn't suggesting anything complex, just a listing page to tell you what's actually applying. You'll forgive me, but I'm quite careful with what I decide is or isn't a tagged service. There are a number of examples in core where someone said "Oh, just make that a tagged service." and I've since been very peeved at my inability to make it do something worth while for end users because of that decision. I'd frankly really like to see a comparison in performance between the two because I see a lot of accusations backed up by exactly 0 links to data.

In short, I'm not suggesting anything complex. I'm favoring the adoption of the most common pattern in core, a potential UI that simply lists what plugins are currently applying (not any sort of configuration) and stats if someone has them.

Eclipse

Is this issue ready for review or are we still working out architectural issues? It's hard to track from the comments, I can mark as Needs work if need be.

Bump, I have the same pending question as #17. We're doing a new release of Panels next week and it would be good to see this change in, if the patch is ready for review.

After discussing with EclipseGC and Phenaproxima, decided that this will go in. If you want a tagged service instead, file a feature request.

Fixed.