-
Notifications
You must be signed in to change notification settings - Fork 71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Media sources are assumed to be entities #1374
Comments
@ajstanley opened up #1273 about this recently. Can we close one of the two? And did you get far into this @ajstanley? |
Heck I'm sorry, I completely missed the other one. My github-search-fu is pretty terrible? I mean we can just close this and reference as a comment on "some investigation was done" |
This was fixed here
<https://github.com/Islandora/islandora/blob/8.x-1.x/src/EventSubscriber/MediaLinkHeaderSubscriber.php#L73-L77>
- way back in the olden days.
This only got the remote object into the system, didn't touch anything that happened after that.
…On Tue, Dec 3, 2019 at 10:40 AM Daniel Aitken ***@***.***> wrote:
Heck I'm sorry, I completely missed the other one. My github-search-fu is
pretty terrible? I mean we can just close this and reference as a comment
on "some investigation was done"
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1374?email_source=notifications&email_token=AADY2JY55SPOLJQABHYYEUTQWZVWFA5CNFSM4JUFRGC2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFZTARI#issuecomment-561197125>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AADY2J2D2MOGDEXC4Y44FJLQWZVWFANCNFSM4JUFRGCQ>
.
--
Alan Stanley
Developer and Training Specialist
Agile Humanities
|
Ok, so I'll close out the old ticket and we'll keep this one to track work to hit up all the other spots where we assume. |
@qadan Are you looking at |
@qadan it seems like Danny was suggesting this feature was fixed. We're not sure if it is - can you re-open with details if this remains an issue? |
Title mostly says it; there appear to be some spots in Islandora functionality working with media which assume that the
source_field
references an entity of some kind, which isn't necessarily the case (i.e. for those sourced remotely).STR
Notes
Specifically, this was observed in a couple places where:
is in use. But
$media->get('whatever_the_source_field_is')
doesn't necessarily return an object implementingEntityInterface
- as in the case of OEmbed sources. In fact, as far as I can tell, there isn't a whole lot of guarantee what comes back from the source field. Sure as heck isn't aMediaSourceInterface
since it gives you the actual file entity.It wouldn't be so much of a problem if an assumption was made that repository items don't reference remote media, but this is breaking any non-entity-sourced media on the site, not just ones used in Islandora; for example, if you wanted to embed a video on the front page from YouTube to introduce your site, you couldn't currently do that. Supporting embedded media keeps us in line with I7 features anyhow.
The text was updated successfully, but these errors were encountered: