Upload module is not a dependency but hook_nodeapi() still depends on it

Ah, sure... sorry. ;-)

Firstly, switching this to a feature request for you, because I didn't realise at time of posting that the Upload module is actually a dependency of FlashVideo, and I've no idea how it came to be disabled?! (I think maybe something I did with Drush, but don't know what.) Because Upload is a dependency by legacy, there would've been no reason to wrap function calls to it in the past, so it makes sense it is how it is now, hence this being a new feature really. =)

I suppose my point is that FlashVideo is no longer tethered to the Upload module with the new CCK feature, so it would be nice to see it dropped as a dependency in the info file so enabling it is optional, along with File Field. But in order for this to happen, we need to check the code doesn't call functions in the Upload module without checking they exist first.

My accidental disabling of Upload exposed one instance of an unnecessary function call in hook_nodeapi() during the 'load' case, line 2070 of flashvideo.module:

         case 'load':
         
            // Get the play counter and increment it...
            $results = db_fetch_object(db_query("SELECT play_counter FROM {flashvideo} WHERE nid = %d", $node->nid));
            if ($results)
            {
            	$node->play_counter = $results->play_counter;
            }
            
            $additions = '';
  // ### THIS LINE ###
            $files = upload_load($node);
            if( count($files) ) {
               foreach( $files as $file ) {
                  if( ($filepath = module_invoke_all('flashvideo_get_file', $file)) ) {
                     $additions['s3files'][$file->fid] = $filepath['file'];
                  }
               }
            }
            
            if( $additions ) {
               return $additions;
            }            
            break;

Simple fix would be something like:

         case 'load':
         
            // Get the play counter and increment it...
            $results = db_fetch_object(db_query("SELECT play_counter FROM {flashvideo} WHERE nid = %d", $node->nid));
            if ($results)
            {
            	$node->play_counter = $results->play_counter;
            }
            
            if (module_exists('upload')) {
              $additions = '';
              $files = upload_load($node);
              if( count($files) ) {
                 foreach( $files as $file ) {
                    if( ($filepath = module_invoke_all('flashvideo_get_file', $file)) ) {
                       $additions['s3files'][$file->fid] = $filepath['file'];
                    }
                 }
              }
            
              if( $additions ) {
                 return $additions;
              }
            }            
            break;

While I don't have a definitive list, this is probably not an isolated instance...? These would need fixing if you did want to remove the dependency. I think it would be nice, as I don't use Upload for anything, so it seems silly to have it enabled and causing system overhead.

Weird! My .info file looks like this:

; $Id: flashvideo.info,v 1.6.2.3 2007/12/29 22:02:13 travist Exp $
name = FlashVideo
description = Provides a way to attach Flash videos to any node type.
dependencies[] = upload
package = FlashVideo
version = VERSION
core = 6.x

; Information added by drupal.org packaging script on 2008-12-05
version = "6.x-1.4"
core = "6.x"
project = "flashvideo"
datestamp = "1228452914"

It's still there?? Looks like it slipped back in by accident...

*slaps head*

Sorry. I got a bit confused - lost track of which version was running where. I'll update to 1.5-rc2, but I'll leave the Upload module enabled for now. =)

Also, set this to bug report again if the dependency is already gone.

Closing - checked the latest code and the line that was causing my problem is gone.

Ps - I worked out what happened here ... I had to use the dev snapshot before 1.5 was released (which probably had the info file with no dependency on Upload, so I disabled Upload thinking I don't need it). At a certain point I reverted FlashVideo, probably having forgotten why it was showing up as changed and not wanting to risk committing stuff I didn't understand, and it went back to 1.4, *with* the dependency.

Sorry for the noise. Nothing to see here! =/