Update Entity translation API usage in Media

Looks good

its now in 7.x-2.x dev
Thanks.

This appears to have caused a regression:
#2856016: Call to undefined method EntityTranslationUserHandler::getActiveLanguage()

going to have to revert this change.

Please see my comments and explanation and suggestions for moving forward in #2856016: Call to undefined method EntityTranslationUserHandler::getActiveLanguage()

Needs work. Please choose a solution that is backwards compatible with those that are not using the Entity translation API , either make it work both ways, or we add a dependency to Entity translation API.

Agreed, we need a new ET release first. Sorry for being too quick to RTBC.

OK sounds good thanks.

To help make this change a successful rollout we should enforce a minimum version of entity translation api in the media.info file.

so I suggest adding this to media.info, piece of cake , so once you've got entity translation api tagged and released we can make this change and document this in the release notes.

ET Beta 6 is out: https://www.drupal.org/project/entity_translation/releases/7.x-1.0-beta6.

Settings to NW to address #13. We should probably update all the other patches around the same way.

Thanks!

@joseph.olstad:

Now that I think about it, ET is only a soft dependency for Media, however by specifying it into the .info file we would make it an actual dependency. I'm not sure we want that.

@plach , I ran some simple tests of this change (the above patch) against the latest version of entity_translation (beta6) and the latest version of media. Appears to behave correctly so-far.

Would be nice for this change to get some mileage in dev.

as for improving this patch, I might suggest that we implement a hook_requirements to check to see if entity_translation is used and then check the version there. Perhaps before committing this to dev, it would be good to add a hook_requirements call for hook_install and some sort of checks to a hook_update to make sure that IF entity_translation is used that it is INDEED at least beta6 or newer.

I like this change and it makes sense. Just haven't had a chance yet to test it or to make the requirements check.

Yeah, good point, I'll post a new patch ASAP

Hi @plach , you'll probably be interested in this hook_requirements for the other modules that have had this change of entity_translation api calls. here is a suggested hook_requirements code that you could use for this.

/**
 * If entity_translation is used, make sure it is beta6 or newer.
 */
function media_requirements($phase) {
  $requirements = array();
  // Ensure translations don't break during installation.
  $t = get_t();
  if (module_exists('entity_translation')) {
    if ($phase == 'update' || $phase == 'install' ) {
      $entity_translation_info = system_get_info('module', 'entity_translation');
      $et_installed_version = $entity_translation['version'];
      $et_installed_datestamp = $entity_translation['datestamp'];
      $march3rd_entity_translation_timestamp = 1488530885;
    
      if ($et_installed_datestamp < $march3rd_entity_translation_timestamp) {
        $requirements['entity_translation']['description'] = $t('Your entity_translation installation version: %version is too old. media requires entity_translation at least beta6 from march 3 2017 or newer.', array('%version' => $et_installed_version));
        $requirements['entity_translation']['severity'] = REQUIREMENT_ERROR;
        $requirements['entity_translation']['value'] = $et_installed_version;
        $requirements['entity_translation']['title'] = $t('Entity translation (when installed) with Media');
      }
    }
  }
  return $requirements;
}

NOTE: I haven't yet tested this code, only partially, I know that the system_get_info works and that the version and datestamp info comes in an array returned by system_get_info, that works. I was thinking this hook_requirements should go into the .install file ? add this to the patch? or would this just go into media.module ?

media_requirements from previous comment needs to be added to the patch, and then tested/reviewed.

Here's the interdiff.

diff --git a/media.install b/media.install
index b6efdd4..d9357c0 100644
--- a/media.install
+++ b/media.install
@@ -122,6 +122,21 @@ function media_requirements($phase) {
     }
   }
 
+  if (module_exists('entity_translation')) {
+    if ($phase == 'update' || $phase == 'install' ) {
+      $entity_translation_info = system_get_info('module', 'entity_translation');
+      $et_installed_version = $entity_translation['version'];
+      $et_installed_datestamp = $entity_translation['datestamp'];
+      $march3rd_entity_translation_timestamp = 1488530885;
+    
+      if ($et_installed_datestamp < $march3rd_entity_translation_timestamp) {
+        $requirements['entity_translation']['description'] = $t('Your entity_translation installation version:
+        $requirements['entity_translation']['severity'] = REQUIREMENT_ERROR;
+        $requirements['entity_translation']['value'] = $et_installed_version;
+        $requirements['entity_translation']['title'] = $t('Entity translation (when installed) with Media');
+      }
+    }
+  }
   return $requirements;
 }

New patch.

reminder to myself to add a check for

isset($entity_translation['datestamp'])

fixed a couple type-o's.
added isset check, could be improved upon.

Needs testing , I ran a test on my own env, but forgot that I already have entity_translation beta6 , so to test that the hook_requirements work someone needs to use an OLD version of entity_translation prior to beta6 , so beta5 et
(make sure entity_translation beta5 is enabled) , test the patch,

expected result: warns you to upgrade to entity_translation beta6 as required so as to prevent people from running the media module with older versions of entity_translation .

 if ($phase == 'update' || $phase == 'install' ) {

Wondering if hook_requirements 'runtime' should be checked in addition to 'update' and 'install'

runtime: The runtime requirements are being checked and shown on the status report page.

$phase == 'runtime'

Fixed in 7.x-2.x dev branch