omeka-s-modules / FileSideload

GNU General Public License v3.0
2 stars 3 forks source link

integrate with CSVImport #5

Closed patrickmj closed 7 years ago

patrickmj commented 7 years ago

Along with rejiggering of CSVImport (develop branch), this should get them working together

zerocrates commented 7 years ago

On the CSVImport side (really this comment should go there but I have this area) I didn't see any alterations to the part that creates the list of valid ingesters (the UI part). That should probably pull from that same config registry (you might need to add stuff like "iiif", "oembed", "youtube" to that list to make that easy... they could all use one same "no-op" adapter class as they don't need special handling).

zerocrates commented 7 years ago

This itself, the integration, looks pretty good. Nice and small.

patrickmj commented 7 years ago

yep, hadn't gotten around to the CSVImport side for those other media before I put together a bunch of test files for those. So, all good that the FileSideload side seems good.

But, on the CSVImport side, since I don't think they need anything special, it seems like these lines handle that basic no-op situation without adding classes that do nothing?

https://github.com/omeka-s-modules/CSVImport/blob/develop/src/Mapping/MediaMapping.php#L47-L50

or am I missing something?

zerocrates commented 7 years ago

Yes, those lines handle the no-op case, but I'm thinking about just how the registry would work if it has to include entries for those ones where no actual behavior change is required (basically what to put in the iiif slot). Just null would probably work too, I suppose.