We need to clean-up our current implementation of the media browser plugins:

  • Let's stop using the CTools plugin system. It gives us no real benefits. What we really wanted is versioning, which is completely separate from the plugin system. We can implement our own media_get_browser_plugins() which checks ctools_plugin_api_info() and invokes hook_media_browser_plugin() and caches it itself. This will also help clean up hook_media_browser_plugin since we only need to specify a 'class' rather than the more complex data that CTools needs when all we need are autoloaded classes.
  • Properly abstract the MediaBrowserPlugin class into an interface (for documentation) and an abstract class. End result will look something like this: http://palantir.privatepaste.com/affdde6e9c This will have a benefit of throwing an automatic error if a plugin class doesn't implement the view() function.
  • Move the access callback into an 'access' method in the plugin.
  • Add a MediaBrowserMenuCallbackPlugin class that easily allows a menu router path to be used as a browser tab plugin (or a MenuBrowserFormPlugin). This could assist #1398896: Decouple Add media page from media browser as our plugins for upload and web could easily be menu router paths of file/add/upload and file/add/url.

Before:

  $plugins['upload'] = array(
    'title' => t('Upload'),
    'weight' => -10,
    'handler' => array(
      'path' => drupal_get_path('module', 'media') . '/includes',
      'file' => 'MediaBrowserUpload.inc',
      'class' => 'MediaBrowserUpload',
    ),
    'access callback' => 'media_access',
    'access arguments' => array('edit'),
  );

After:

  $plugins['upload'] = array(
    'title' => t('Upload'),
    'weight' => -10,
    'class' => 'MediaBrowserUpload',
  );
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Dave Reid’s picture

Priority: Normal » Major
Dave Reid’s picture

Status: Active » Needs review
FileSize
9.5 KB

First phase: implement the first part: stop using CTools plugin system.

Dave Reid’s picture

Patch in #2 has been committed to Git.
http://drupalcode.org/project/media.git/commit/8670623

DamienMcKenna’s picture

Status: Needs review » Fixed

I think that means it's fixed? :)

Dave Reid’s picture

Status: Fixed » Active

No, I still have to address parts 2-4 of the original issue now. :)

Dave Reid’s picture

Status: Active » Needs review
FileSize
11.83 KB

Part 2, defining a proper interface and an abstract class.

Dave Reid’s picture

Status: Needs review » Active
Dave Reid’s picture

Status: Active » Needs review
FileSize
10.89 KB

Here's part three which implements MediaBrowserPluginInterface::access() on all the tabs. Also helps cleanup the Views tab code by re-using $this->view.

Dave Reid’s picture

Status: Needs review » Fixed

Committed phase three with http://drupalcode.org/project/media.git/commit/0c3e982. I'm going to consider this fixed for now. I'll make a separate issue for the MediaBrowserMenuPath plugin.

Status: Fixed » Closed (fixed)

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