Not sure how to do this, but the vod importer won't work without at least one of the media_youtube, media_vimeo, media_cloudcast, etc modules enabled

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

CHourihan’s picture

Issue summary: View changes
FileSize
624 bytes

Hey there,

I went and made a patch to help this issue out. This patch makes it so that a warning message is given on the import page saying there must be a media_x module installed. I'm not entirely sure this message was in the right spot and it currently shows no matter what, even if the modules are installed. I'll keep working on getting it situational and any feedback on the message location would be great. Also let me know if the patch wasn't created right, tis my first one.

Thanks,
~Chuck

kreynen’s picture

Status: Active » Needs work

The patch is created correctly and technically gets the job done, but this message will always show up. What about using an approach more like FullCalendar's install check for the

http://drupalcode.org/project/fullcalendar.git/blob/refs/heads/7.x-2.x:/...
http://drupalcode.org/project/fullcalendar.git/blob/refs/heads/7.x-2.x:/...

the _fullcalendar_status() uses...

if (!file_exists($path)) {
  return FALSE;
}

Instead of checking for a specific file, we could loop through an array of media_ modules cm_vod_feed might be use with using module_exists(). If none of these modules are enabled when you try to enable cm_vod_feed, it will fail with error like what you added.... but once the user enables any of those media_[provider] modules, the user will be able to enable cm_vod_feed.

Make sense?

CHourihan’s picture

FileSize
836 bytes

Hi again.

I have a new patch which has an if else statement checking if the media modules are enabled. Right now it only checks for youtube, vimeo, and cloudcast but I can easily add the rest, if you could list the relevant ones that'd be great. The warning message only displays when module_exists() returns false for all the modules listed.

kreynen’s picture

Really sorry I didn't see that you updated this patch until now.

This check needs to be in a hook other than init. That get's called on every page load so it's checking way too often even after the modules exist. Every time a user clicks something Drupal would be asking...

Are media_ modules enabled? Yes. Ok, do nothing.
Are media_ modules enabled? Yes. Ok, do nothing.
Are media_ modules enabled? Yes. Ok, do nothing.
Are media_ modules enabled? Yes. Ok, do nothing.
Are media_ modules enabled? Yes. Ok, do nothing.

That doesn't really cost us a lot in terms of CPU or RAM, but it all adds up. If every module was doing that, Drupal would get very slow. Instead I moved the function you wrote to the .install so that it is only checking on install and on the status page.

There are still a few issues with this patch.

1) The warning shows up above the status report in addition to in the status report
2) We are checking for media_archive twice and aren't checking for dozens of other media_ modules

If you can get those issues resolved, I can commit this and you'll get your first commit credit on your user profile https://drupal.org/user/2734505.

kreynen’s picture

Issue tags: +cmd cmd-get-started
kreynen’s picture

CHourihan’s picture

Hello

I think I've got it working as intended. I added a bunch of the media_x modules to the conditional and fixed the redundant message on the admin status report. Let me know if there are any more issues with it.

Thanks,
Chuck

kreynen’s picture

Status: Needs work » Fixed

Status: Fixed » Closed (fixed)

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