Followup of #229028: Media section in admin.

CommentFileSizeAuthor
patch.txt5.07 KBcasey

Comments

casey’s picture

casey commented

Ow wrong title, first I wanted to suggest splitting the mapi into the actual mapi module and an ui module.

rhys’s picture

rhys commented
Status: Active » Needs work

I like the idea of splitting the mapi into two modules, one for the administration, which can be setup, and the other for the general functioning. So that's not a problem, we'll do that in the next module version.

The patch, although it deals specifically with the menu items themselves, does not provide a way to deal with the forms themselves, which take arguments dependent on the location (i.e. arg(4) == 'derivatives'), and since now that's changed, changing the module would involve changing literally all the references to administration processes throughout all the modules that utilize MAPI.

It'll end up needing some more thought, so I'll look into it before I begin really trying to code something.

casey’s picture

casey commented

Ow didn't see that.

These args should be passed using the 'callback arguments' property hook_menu(). Maybe you should use the regular menuitem structure in hook_media_admin() (also gives back the possibilty not to use drupal_get_form)

A solution is to define a constant containing the length of basepath of media-pages. Something like this:

define('MEDIA_ADMIN_PATH', 'admin/media');
define('MEDIA_ADMIN_ARG', 2);
define('MEDIA_PATH', 'media');
define('MEDIA_ARG', 1);

// define a menu item
    $items[] = array(
      'path' => MEDIA_ADMIN_PATH .'/extensions',
      'title' => t('Extensions'),
      'description' => t('Lists and manages the extension'),
      'callback' => 'drupal_get_form',
      'callback arguments' => array('media_admin_extensions_form', arg(MEDIA_ADMIN_ARG+1)),
      'access' => user_access('administer media'),
    );

function media_admin_extensions_form($ext) {
  $exts = media_extension_list();
  // ...
}
rhys’s picture

rhys commented

I like the constants provided, since they allow for configurable URL locations for the media section. One of the reasons I was trying to simply the menu system, was to wipe out a lot of extra text, which takes up precious script memory. Unfortunately I think I sacrificed readability while I was at it.

I'll provide the menu items exactly the same as in the hook_menu, since learning a different way of doing them seems rather a burden for people already familiar with the system.

I think in general the system that you've got worked out here will work well.

I'm currently implementing the administration section as part of mapi_ui, and the rest will be part of mapi.

rhys’s picture

rhys commented
Status: Needs work » Fixed

Ok, so this will be coming out in the upcoming update, so I'm going to consider this fixed.

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

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