Opencast-Moodle / moodle-block_opencast

Block to manage Opencast publications in moodle
22 stars 27 forks source link

Deletion of other external lti tools #346

Closed berthob98 closed 10 months ago

berthob98 commented 10 months ago

We experienced a problem where block_opencast deletes the database entries of other external lti tools.

release = 'v4.1-r1'; version = 2023052600;

The block wrongfully deletes entries in the mdl_lti table in the moodle database. We include Matlab Grader as an external lti tool, thereby entries in the mdl_lti table are written.

The regular execution of the block_opencast\task\cleanup_lti_module_cron task leads to the deletion of these entries.

More specific the cleanup_lti_module_entries function deletes these entries in line 1369 and 1371.

I didn't dig deep enough in to the purpose of the task yet, but to my understanding there should be a check preventing the deletion of non opencast lti modules.

I hope this is helpful and my understanding of the problem is correct thereby preventing others from experiencing this problem.

NinaHerrmann commented 10 months ago

@ferishili Any ideas on that? I saw that you were committing the function - however, I can not see any changes on the mdl_lti table. There might be some wrong processing of IDs?

ferishili commented 10 months ago

Hi @NinaHerrmann, @berthob98, I was not able to reproduce this issue. And based on the fact that it validates that module in 4 tier before deleting it, makes me think, whether you have selected the right LTI module in your Opencast Block settings for this! Because, as you can see, it does not randomly delete modules, but it gets its module id from the table that is related to opencast block setting which holds those infos. (so my first assumption here is misconfiguration!) So I would suggest that you:

  1. check if the LTI module settings in block_opencast settings are correct (for both series and event)
  2. provide (if possible) visual scenarios/more details about your settings etc.

And as you recommend, that is great if we also check that module is opencast LTI, I am afraid those 4 tier validations are intended exactly to do so! If you have something more to check against to improve those validations, please add as a PR.

BR

berthob98 commented 10 months ago

@ferishili Thank you for your help. LTI Module Settings (addltienabled_1 & addltiepisodeenabled_1) are both disabled. When disabled the settings are collapsed: image

Only when I enable the settings the other options are expanded, which allow me to select the addltipreconfiguredtool_1 which indeed is MATLAB the tool that gets deleted and obviously the wrong option. The dropdown doesn't allow me to choose any other options, because it is the only extarnal preconfigured LTI module we have.: image

Does the block_opencast\task\cleanup_lti_module_cron have to run when addltienabled_1 is false? Maybe disabling the task if addltienabled_1 is false could prevent this behavior?

ferishili commented 10 months ago

I need to check the top level of that cron job, I would say and assume, there is something missing there!

NinaHerrmann commented 10 months ago

Thanks for checking so fast @ferishili ! :smile: Can you estimate when you will have time to look at it? We are hoping to release by the end of this week or next week and it would be great to have a fix included ! :smile:

ferishili commented 10 months ago

No problem, I will create the PR during these 2 days!

ferishili commented 10 months ago

Hi, the PR is ready, please try to test it through, because I wasn't fully focused back to Moodle environment 😄.