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.

Support from Acquia helps fund testing for Drupal Acquia logo

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.