search

opencast-ilias / OpenCast

Opencast plugin for ILIAS LMS. The plugin shows a Opencast series as an ILIAS object. This version of the ILIAS plugin for Opencast is operated and developed collaboratively by a community. The University of Bern acts as coordinative maintainer.
GNU General Public License v3.0
13 stars 14 forks source link

[fix] #374 response from OC-16 changed #375

Open okaufman opened 4 days ago

okaufman commented 4 days ago

This PR fixes this issue #374. The responce from the OpenCast Version 16 API has changed from the response with OC-Version 15. The changes can also be seen in the docu: https://docs.opencast.org/r/16.x/admin/upgrade/#api-changes

There is no 'search-results' being returned in OC16. So i check if that arraykey 'search-results' exists to determine if the version is older than 16.

dagraf commented 2 days ago

Our tests wer successfull.

Therefore and @chfsx: Can you please review this PR and merge it if everything is fine or comment it, if changes are needed? Thx!

ferishili commented 1 day ago

I am not aware of the context here, but as reminder, I should suggest to rely on Opencast PHP Library, because it is mainly purpose of that library to overcome these differences as much as possible. I am not sure if it could be applied here, but always thing of backward compatibility!

chfsx commented 1 day ago

@ferishili Yes absolutely! The problem here is probably that the API itself is not different, but the response is (the nesting of the arrays is different), so

$episode_data = $this->api->routes()->search->getEpisodes(...

does not return the same result depending on the opencast version. I don't know if you can or should catch this at the library level (same problem as here). The problem would probably be to build this switch into the library so that you not only have to abstract the routes, but also actually have to abstract the responses. In addition, there would then ideally be version-specific implementations in the library. I'm open to any variant, the switch must be somewhere as it seems :-) unless there is a bug in opencast 16 that the response is different there, I just wanted to rule that out before we built in such a switch.

reiferschris commented 1 day ago

Please note that this part of opencasts API is not the "external" api /api but the /search index that we only use for livestreams in the plugin. The way the livestreams are published relies on this search index which is why it is the only place where we (have to) use it, even though it is a pain.. I'm a huge fan of solving this in the library as it is designed to unify stuff like this.

This isn't a bug but an effect of some refactoring as @okaufman already posted. https://docs.opencast.org/r/16.x/admin/#upgrade/#api-changes

ferishili commented 1 day ago

I have created the related issue there. However, this could get solved there with some delay, therefore I would suggest going on with this fix for now.