Skip to content
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

Closed
qadan opened this issue Dec 2, 2019 · 6 comments
Closed

Media sources are assumed to be entities #1374

qadan opened this issue Dec 2, 2019 · 6 comments

Comments

@qadan
Copy link

qadan commented Dec 2, 2019

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

  • Create a new media type
  • As its 'Media source', use 'Remote video'
  • Create a media of that media type, using a YouTube video or some other valid source.
  • Try to edit that media
  • Some errors come up

Notes

Specifically, this was observed in a couple places where:

$media->get('whatever_the_source_field_is')->referencedEntities();

is in use. But $media->get('whatever_the_source_field_is') doesn't necessarily return an object implementing EntityInterface - 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 a MediaSourceInterface 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.

@dannylamb
Copy link
Contributor

@ajstanley opened up #1273 about this recently. Can we close one of the two?

And did you get far into this @ajstanley?

@qadan
Copy link
Author

qadan commented Dec 3, 2019

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"

@ajstanley
Copy link
Contributor

ajstanley commented Dec 3, 2019 via email

@dannylamb
Copy link
Contributor

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.

@dannylamb
Copy link
Contributor

@qadan Are you looking at 1.0.0 or are you running from HEAD?

@kstapelfeldt
Copy link
Member

@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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants