Problem/Motivation

I have defined a custom AssetMediaSource plugin. Now I'm getting a fatal error when trying to edit content.

TypeError: Drupal\acquia_dam\EmbedCodeFactory::getSelectOptions(): Return value must be of type array, null returned in Drupal\acquia_dam\EmbedCodeFactory::getSelectOptions() (line 109 of /app/docroot/modules/contrib/acquia_dam/src/EmbedCodeFactory.php).

which is accompanied by a warning:

Warning: Undefined array key "[my AssetMediaSource plugin id]" in Drupal\acquia_dam\EmbedCodeFactory::getSelectOptions() (line 109 of /app/docroot/modules/contrib/acquia_dam/src/EmbedCodeFactory.php)

Steps to reproduce

Define a custom AssetMediaSource and then do something that calls EmbedCodeFactory::getSelectOptions()

Proposed resolution

Long term, there needs to be some way to alter or affect how EmbedCodeFactory::getSelectOptions() works. It has a hardcoded list of AssetMediaSource ids that I don't see any way to alter.

Short term, it might be enough to change the return form:

return $asset_type ? $mapping[$asset_type] : $mapping;

to something like

return $asset_type ? ($mapping[$asset_type] ?? $mapping['General']) : $mapping;

That $mapping['General'] is defined earlier and looks like it's never used. This is probably how it was supposed to be used.

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork acquia_dam-3565081

Command icon 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:

About issue forks

Comments

danflanagan8 created an issue. See original summary.

danflanagan8’s picture

danflanagan8
Primary language English
Location St. Louis, US
commented
Related issues: +#3527118: Support more asset_search_key values for Asset plugin

danflanagan8 opened merge request !239

danflanagan8’s picture

danflanagan8
Primary language English
Location St. Louis, US
commented

I opened an MR with a test that triggers the bug. It calls EmbedCodeFactory::getSelectOptions('assettestmediasource') and gets this output:

There was 1 error:
1) Drupal\Tests\acquia_dam\Kernel\EmbedCodeFormatterTest::testGetSelectOptions
TypeError: Drupal\acquia_dam\EmbedCodeFactory::getSelectOptions(): Return value must be of type array, null returned
/builds/issue/acquia_dam-3565081/src/EmbedCodeFactory.php:109
/builds/issue/acquia_dam-3565081/tests/src/Kernel/EmbedCodeFormatterTest.php:499
--
1 test triggered 1 PHP warning:
1) /builds/issue/acquia_dam-3565081/src/EmbedCodeFactory.php:109
Undefined array key "assettestmediasource"
Triggered by:
* Drupal\Tests\acquia_dam\Kernel\EmbedCodeFormatterTest::testGetSelectOptions
  /builds/issue/acquia_dam-3565081/tests/src/Kernel/EmbedCodeFormatterTest.php:496
danflanagan8’s picture

danflanagan8
Primary language English
Location St. Louis, US
commented
Status: Active » Needs review
Related issues: +#3539187: EmbedCodeFactory::getSelectOptions performance improvements

I made what's probably the smallest reasonable change that prevents this error.

The more look at this class, though, it really seems like code that should get absorbed into the asset type plugins. The plugins themselves should know what embed options they support, no?

I see there's an open issue to improve performance (#3539187: EmbedCodeFactory::getSelectOptions performance improvements). I wonder if it's being discussed to scrap this class entirely and instead add some methods to MediaSourceTypeInterface and/or AssetMediaSourceManager.