Problem/Motivation
In #3506556: Allow for extending supported asset types new Asset plugins were added to allow a more extensible and customizable integration with the DAM.
The two important plugin propeties are:
/**
* The search key passed to DAM.
*
* @var string
*/
public $asset_search_key;
/**
* The search value passed to DAM.
*
* @var string
*/
public $asset_search_value;
This looks promising, but at the moment there really are only two values of asset_search_key that are supported. They are ft and ff. The acquia_dam_asset_type Views argument and the acquia_dam_asset_import queue appear to support any asset search key that is valid with Widen search. However, the MediaTypeResolver has ff and ft hardcoded in a mapping that can't be altered in any way.
$mapping = [
'ft' => 'format_type',
'ff' => 'format',
];
So that's where the trouble comes in.
Steps to reproduce
Use an alter hook to add a new plugin definition that uses some other search key. In this case, we use a metadata property with the key assetType.
/**
* Implements hook_acquia_dam_media_source_info_alter().
*/
function my_module_acquia_dam_media_source_alter(&$definitions) {
if (isset($definitions['image'])) {
$definitions['icon'] = $definitions['image'];
$definitions['icon']['id'] = 'icon';
$definitions['icon']['label'] = 'Icon';
$definitions['icon']['asset_search_key'] = 'assetType';
$definitions['icon']['asset_search_value'] = 'Icon';
}
}
Thus the icon Asset plugin is just like the image plugin but it searches using the assetType metadata instead of the format_type file property.
Proposed resolution
I would like the MediaTypeResolver to try its best to use search keys that are not in the hard coded mapping. The resolver would look for the key in both the asset's file_properties and its metadata.
Remaining tasks
User interface changes
API changes
Data model changes
Issue fork acquia_dam-3527118
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
danflanagan8Comment #4
danflanagan8I put in an MR. Setting to NR.
Comment #6
patrickfwestonThis is going to be really necessary for a client I'm working with. They have asset types that support both Word/Excel documents and PDFs. I'm having trouble at the moment given they have different AssetMediaSource instances, using both the
DocumentsandPdftypes.Comment #7
japerryI think this is on the right path, would like some tests though. Probably should look at how we're hard coding the mappings as well, so you don't need to use a hook_alter.
Comment #8
danflanagan8Hi, @japerry
The hook_alter is required in my case because all the asset plugin classes are final, not so much because the mappings are hardcoded. If the image plugin weren't final I probably would have extended it to create my new icon plugin, but I think the hook is fine here.
Yeah, I think for supporting a single asset_search_key and a single asset_search_value, the MR is pretty effective. One problem that still comes up for my image/icon example is that any asset that Drupal resolves as an icon must all resolve as an image. So it's vital that the MediaTypeResolver tries to resolve icons before it tries to resolve images. I'm pretty sure the resolver iterates through the media types in alphabetical order (by media type machine name). Luckily for us, our icon media (which uses our icon asset type) comes before our image media (using the image asset type). That's pretty darn lucky though!
TL;DR; It would be nice to be able to set the weight of a media type as returned by
MediaTypeResolver:: getMediaTypes.But as pointed out by @patrickfweston in #6, there are good use cases for supporting more than one asset_search_value. Perhaps the asset_search_value property should accept an array or a comma-delimited string of values to OR. I'd really appreciate thoughts on that possibility. One notable difficulty is that such a feature would require changes to the AssetQueueService and the AssetTypeArgument classes.
Comment #9
danflanagan8Tests added.
Comment #11
japerryLooks good, and thanks for the test! Committed.