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.

Members fund testing for the Drupal project. Drupal Association Learn more

Comments

Dave Reid’s picture

Status: Active » Needs review
FileSize
961 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
FileSize
1010 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 :

  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 :

  /**
   * 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 :

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
FileSize
832 bytes

;) Here's a patch for #8

ttjordan81’s picture

FileSize
1.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

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.