Problem/Motivation
A media item always references a source, and we frequently want to access field values from that source in MediaSource plugins. Instead of repeating similar code in many places, it will be easier to write, easier to read, and more reliable to provide a method for this common task.
The code for this was already introduced in #2831944: Implement media source plugin for remote video via oEmbed. We are splitting off this issue in the hope of making the new method available a little sooner.
Proposed resolution
Add the method getSourceFieldValue() to MediaSourceInterface and implement it in MediaSourceBase.
Completed tasks
- Add the method to the interface.
- Add the method to the base class.
- Add a test.
Remaining tasks
- Write a change record.
User interface changes
None
API changes
Add a new method to MediaSourceInterface.
Most classes that extend MediaSourceBase will not have to make any changes.
- Any class that implements the interface without extending the base class will have to implement this method.
- If the source value is not the main property of the source field, as returned by
mainPropertyName(), then the MediaSource plugin will need to override this method.
Without this method, we use code like this in MediaSource plugins:
$source_field = $this->getSourceFieldDefinition($media->bundle->entity)->getName();
/** @var \Drupal\Core\Field\FieldItemInterface $source_field_item */
$source_field_item = $media->get($source_field)->first();
$main_property = $source_field_item->mainPropertyName();
$resource_url = $source_field_item->get($main_property)->getString();
After adding this method, that can be simplified:
$resource_url = $this->getSourceFieldValue($media);
Data model changes
None
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | interdiff-2934966-5-9.txt | 716 bytes | starshaped |
| #9 | 2934966-9.patch | 2.63 KB | starshaped |
| #5 | 2934966-get-source-value-5.patch | 2.75 KB | benjifisher |
| #5 | interdiff-2934966-3-5.txt | 668 bytes | benjifisher |
| #3 | 2934966-get-source-value-3.patch | 2.78 KB | benjifisher |
Comments
Comment #2
benjifisherThe attached patch is based on the code from #2831944: Implement media source plugin for remote video via oEmbed, with minor changes to the comments.
I will start working on a test.
Comment #3
benjifisherThe attached patch fixes the method:
return $field_item->{$field_item->mainPropertyName()};instead ofreturn $field_item->get($field_item->mainPropertyName());.It also adds a test, so I think this patch is ready for review.
Comment #4
phenaproximaThanks, @benjifisher! This looks fantastic.
This will return mixed, not TypedDataInterface.
Comment #5
benjifisherThanks for checking! The attached patch changes the @return to "mixed" and also tweaks the summary line of the docblock.
Comment #6
phenaproximaLooks great! RTBC once Drupal CI passes it. This is also a big win for developers who are using Media, so I'm tagging this as a DX improvement.
Comment #7
benjifisherComment #8
larowlanI don't think we should speculate on the % or elude to basic language features like inheritance.
So tl;dr - not sure this comment is needed
Comment #9
starshapedRemoved the comment as noted in #8.
Comment #11
larowlanCommitted as bfe514a and pushed to 8.5.x.
Do you want to add a tag here for tracking progress in this weekend's sprint?
Publishing change record.
Comment #13
james hawthorn-byng commentedJust in case someone else comes here is is as equally as confused as I was as to how we can use this on a Media object...
If you have a media object and you want to get the source field value you need to do
$mediaSource = $media->getSource()then$mediaSourceValue = $mediaSource->getSourceFieldValue()Every time I tried to see what was coming back in just $media->getSource() my site crashed or I wasn't allowed to kint out the protected properties so I just wanted to put a little explanation here.
Comment #14
benjifisher@James Byng:
That sort of usage information is in the change record (CR). If you check the right sidebar for this issue, you will find a link to the CR getSourceFieldValue() method added to MediaSourceInterface, which says pretty much the same thing as your comment.