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

Dave Reid’s picture

Status:Active» Needs review
StatusFileSize
new961 bytes
aaron’s picture

Status:Needs review» Reviewed & tested by the community

works great!

aaron’s picture

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.

ksenzee’s picture

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.

mpotter’s picture

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

aaron’s picture

Status:Needs review» Fixed

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

nikosnikos’s picture

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);

 
//...
}
?>
aaron’s picture

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.

aaron’s picture

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

nikosnikos’s picture

Status:Needs work» Needs review
StatusFileSize
new832 bytes

;) Here's a patch for #8

ttjordan81’s picture

StatusFileSize
new1.72 KB

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

aaron’s picture

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

nikosnikos’s picture

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.

ddyrr’s picture

for alpha5, according to #14

Dave Reid’s picture

StatusFileSize
new837 bytes

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

Dave Reid’s picture

Status:Needs review» Fixed

Status:Fixed» Closed (fixed)

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