Islandora-Labs / islandora_solution_pack_oralhistories

Adds all required Fedora objects to allow users to ingest and retrieve Oral Histories (video/audio) files through the Islandora interface
GNU General Public License v3.0
13 stars 23 forks source link

Check if media_dsid is set before using. #124

Closed jordandukart closed 6 years ago

jordandukart commented 6 years ago

What does this Pull Request do?

Another case where a derivative may not be present til later, or derivative generation fails with "Play OBJ datastream if MP4 is not present" being unchecked.

What's new?

Conditional check.

How should this be tested?

Ingest an object with deferred derivative generation set and the "Play OBJ datastream if MP4 is not present" unchecked within the configuration for the video SP.

MarcusBarnes commented 6 years ago

@jordandukart Thanks so much for this. I've just realized that we don't have smoke tests for when using this module with the "deferred derivative generation" set. This is related to how we and others have used the module thus far. So you're contribution is much appreciated. I'll create an issue to add testing with "deferred derivative generation" set to be included in the testing spreadsheet for the next release.

jordandukart commented 6 years ago

Cool thanks. Would you guys be open to a coder pass if I get around to it?

MarcusBarnes commented 6 years ago

@jordandukart That would be awesome. Definitely something on our to-do list in any case.

MarcusBarnes commented 6 years ago

@jordandukart Is the expectation not to get an error, warning or notice when, for example, uploading an .mov video and transcript file? Or do you expect the player to play the .mov file before the MP4 derivative is created?

jordandukart commented 6 years ago

If it's the deferred derivative route the page can load before the derivative is made so the warning about not being able to play the content seems valid?

MarcusBarnes commented 6 years ago

Just confirming that I understood the expectations properly. Your pull-request does eliminate the PHP notices that result when deferred derivatives generation is on. I'll merge momentarily. Thank you again for this.