We should allow users to re-use YouTube videos in multiple places using the 'Web' media browser plugin rather than throwing an error saying the URL is already in the library.

This depends on #1121808: Change file_uri_to_object to re-use files by default and remove duplicate validation checks in handlers landing in Media first.

Comments

Status:Active» Needs review
StatusFileSize
new961 bytes

Status:Needs review» Reviewed & tested by the community

works great!

Status:Reviewed & tested by the community» Fixed

and committed. thanks! btw, i added you as a maintainer to this module, Dave.

Status:Fixed» Closed (fixed)

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

Status:Closed (fixed)» Needs review
StatusFileSize
new1010 bytes

This is broken for users of media 1.x, where file_uri_to_object() doesn't reuse existing file objects by default. I'm not sure if media_youtube is still willing to support media 1.x, but if so, here's a patch. Note that this patch also sets the file object's timestamp to the request time, so that when someone reuploads a video, it shows up as the newest media item at the top of the admin list.

In #5 you refer to "$file_obj->timestamp". Shouldn't this be "$file->timestamp" ??

Status:Needs review» Fixed

got that with mpotter's additional fix. Thanks guys.

I know this is fixed but actually I think there's no need to override the save() method in MediaInternetYouTubeHandler :

  1. This method is overridden to put a timestamp to the file, this is done in the file_save() function called just after that.
  2. The original save() method (from MediaInternetHandler) call a preSave() method that have to be overriden in children to do additional operations, before the file has been saved

MediaInternetYouTubeHandler :

<?php
 
public function save() {
   
$file = $this->getFileObject();
   
// If a user enters a duplicate YouTube URL, the object will be saved again.
    // Set the timestamp to the current time, so that the media item shows up
    // at the top of the media library, where they would expect to see it.
   
$file->timestamp = REQUEST_TIME;
   
file_save($file);
    return
$file;
  }
?>

MediaInternetHandler :

<?php
 
/**
   * Saves a file to the file_managed table (with file_save)
   *
   * @return StdClass
   */
 
public function save() {
   
$file_obj = $this->getFileObject();
   
$this->preSave($file_obj);
   
file_save($file_obj);
   
$this->postSave($file_obj);
    return
$file_obj;
  }
 
/**
   * After the file has been saved, implementors may do additional operations.
   *
   * @param $file_obj;
   */
 
public function postSave(&$file_obj) {
  }
 
/**
   * Before the file has been saved, implementors may do additional operations.
   */
 
public function preSave(&$file_obj) {
  }
?>

file_save :

<?php
function file_save(stdClass $file) {
 
$file->timestamp = REQUEST_TIME;
 
$file->filesize = filesize($file->uri);
 
//...
}
?>

Status:Fixed» Needs work

Good points. However, file_save only adds a timestamp if not already present; we're actually potentially altering an already existing timestamp. But overriding ->presave would certainly simplify the code.

Oh, whoops, I stand corrected. I missed your last snippet.

Status:Needs work» Needs review
StatusFileSize
new832 bytes

;) Here's a patch for #8

StatusFileSize
new1.72 KB

Here is the combination of the two patches... #1 and #5. Anyone still using alpha 5...

This should be tested against both branches of the media module.

Actually ans as I said in #8 there's no need to override the save() method in MediaInternetYouTubeHandler

The patch in #11 should work and allow to reuse youtube videos without overriding save() method.

for alpha5, according to #14

StatusFileSize
new837 bytes

Ok so I think this is what needs to be committed to 7.x-1.x still. Will test manually today.

Status:Needs review» Fixed

Status:Fixed» Closed (fixed)

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