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

Fixes Opencast-Moodle/moodle-filter_opencast#54 #55

Open
wants to merge 1 commit into
base: MOODLE_405_STABLE
Choose a base branch
from

Conversation

elke-hsh
Copy link

No description provided.

Copy link
Contributor

@ferishili ferishili left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@elke-hsh Thanks for your work. That would be great if you could take care of these points.
Kudos for using the organization api endpoint to retrieve the engage UI URL!

@@ -24,8 +24,11 @@

namespace filter_opencast\local;

use core\check\performance\debugging;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this if it is not in use!

// Endpoint to call the engage.ui url of presentation node.
// Make sure the api user has the rights to call that api endpoint.
$engageurlendpoint = '/api/info/organization/properties/engageuiurl';
$result = json_decode($api->oc_get($engageurlendpoint), true);
Copy link
Contributor

@ferishili ferishili Jan 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use opencastapi (Opencast PHP Library) property: getOrgEngageUIUrl
So that would be something like this:
$api->opencastapi->baseApi->getOrgEngageUIUrl();

api->oc_get is no more recommened and it will be removed in near future!

@@ -35,3 +35,4 @@
$string['setting_uselti_desc'] = 'When enabled, Opencast videos are delivered through LTI authentication using the <strong>default Opencast video player</strong>. This is typically used alongside Secure Static Files in Opencast for enhanced security.';
$string['setting_uselti_nolti_desc'] = 'To enable LTI Authentication for Opencast, you must configure the required credentials (Consumer Key and Consumer Secret) for this instance. Please do so via this link: {$a}';
$string['setting_uselti_ocinstance_name'] = 'Opencast API {$a} Instance';
$string['no_engageurl_error'] = 'No URL available. Please check if the api endpoint is available and if the api user has the proper rights.';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to use a more comprehensive (not-technical) info here, as it might be displayed to the end-users and they for example cannot check the api or the user rights!
What do you think?

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

Successfully merging this pull request may close these issues.

2 participants