Backup/Import improvements (series/episode selection + modules fix)

[ferishili]

This PR fixes #193

Description

Currently there is no possibility to select series and episode in course import wizard when selection "Duplicate Event" as import mode, and then the entire series and its events in a course will be imported. Apart from that, after the import is completed if you have also imported LTI modules or Activity modules, they also get imported in the new course, but they are still pointing to the event or series from the source course that everything has been imported from.

How it works.

• In course import wizard, (backup stage) you can now select/deselect series/events when "Duplicate event" as import mode is used.
• In course import wizard, (restore stage) there is no selection possibility, why? because having to select from source in the last (backup stage) does the job, and repeating that is an overkill!
• A new db table is introduced to hold the import mapping data called "block_opencast_importmapping"
• (During restore process): we collect mapping records including series and event ids.
• (After restore process):
  • Series will be fixed in this stage, because there is no need to wait for any workflow to do the duplication process.
  • Events: for events now the import mapping gets the flag that indicated the restore process is completed, why? because, we make sure that all modules including LTI and activities are imported.
  • In duplicate event adhoc task, when the workflow is finished successfully, we then record the workflow id in import mapping record of the event and we schedule another additional adhoc task called "process_duplicated_event_module_fix", which is responsible to get the duplicated event id and run the lti and activity module fix.

How does the fix methods work:

the whole concept is that the newly created course after import contains modules that has series/event id from the old course. Therefore, the only goal is to collect the newly created series id, or duplicated event id and replace them in the modules (if any) of newly created course.

NOTE

• The PR also includes in-depth fixing module feature in PHPUnit tests (backup_test.php)
• Because we need to fix both LTI and Activity (mod) modules, we need to the fixing here in block_opencast!
• The version is bumped to add new db table as well as new adhoc task.
• In some places, I have added my name as the author, because the whole thing has been change completed in those files/classes.

Please refer to #193 (comment)
This PR will fix: Opencast-Moodle/moodle-mod_opencast#34 as well

[ferishili]

Hi @mwuttke,
Please test this PR again, and write your feedback, so we can proceed with merging it.
Thanks in advance.

[mwuttke]

@ferishili: My tests are successful. I made a bulk import with a new course and one with an old reseted course, using the duplicate workflow, and it works as expected.

We used for the test Opencast Version 14.4/15.3 and Moodle Version 4.1.9+.

[justusdieckmann]

Hey @ferishili,

thank you very much!! I'm sorry for the wait, I have been rendered a bit ineffective for the last few weeks by a kidney stone...

I've left some notes, some more important than others :)

Justus

[ferishili]

Hi @justusdieckmann,
Sorry to hear that, and hope you've healed and are well now.

Thanks for your review, I will try my best to apply the changes as soon as possible.
Best Regards
Farbod

[ferishili]

Hi @justusdieckmann,
Thanks again for the review, the changes are applied, and open (unresolved) changes are waiting for your reply.

Best Regards

[justusdieckmann]

Hi @ferishili,

thank you, I replied to all comments!

Kind Regards
Justus

[ferishili]

Hey @justusdieckmann,
the changes are there, thanks a lot.
Hope they work fine, and I did not miss something!

BR

[ferishili]

Hey @justusdieckmann, Is there something else left to do here?

[justusdieckmann]

Could you look at this in Moodle 4.4? I always get the error 'Your settings have been altered due to unmet dependencies' but can't find the specific cause.

[ferishili]

Hi @justusdieckmann,
as I mentioned this in last community meeting, this PR was promised and delivered only for Moodle up to 4.3!
Let's merge this PR first and then we tackle the issues regarding refactoring for Moodle 4.4 in separate issue.

[NinaHerrmann]

Hey @ferishili as said in the meeting we wanted to have the PR for the 4.4 Release. Therefore, we need it to be 4.4 ready.

[justusdieckmann]

Hey @ferishili,

if it would just be refactoring and code style fixes, I would agree. I actually was holding off creating and merging the code style fixes for 4.4 in order to not create more hassle and merge conflicts for you, since 4.4 for example wants to have the language strings sorted alphabetically by key.

But this seems to be a functional bug which completely prevents imports in 4.4. Moodle 4.4 has been released for a few weeks now and we want to release a 4.4 version as soon as possible, so if we already found this bug here, it really doesn't make sense to delay fixing it

[justusdieckmann]

Hey @ferishili,

I've added a fix, does it look good to you?

[ferishili]

Hey @justusdieckmann,
Thanks, I have tested the fix for Moodle 4.4, unfortunately it throws error stating error/setting_locked_by_config in Moodle 4.1!
@mwuttke would you please test this on your test instance (Moodle 4.1 if I am not mistaken)?

[justusdieckmann]

Hey @ferishili,

that's weird, I have tested it on Moodle 4.1 and no error occured

[mwuttke]

Hello @ferishili,

I was also unable to detect any errors during my tests with Moodle 4.1 of this pull request.

[ferishili]

My scenario that ends up with that error: (Moodle 4.1)

• Latest PR changes
• In a course overview page -> Course reuse
• Import as mode selected
• Select a course that has Opencast Videos with a few published videos
• Click on next
• Error: error/setting_locked_by_config

[ferishili]

As video:

Screen.Recording.2024-05-15.at.16.15.47.400.mov

[justusdieckmann]

Do you have a stacktrace for me @ferishili?

[ferishili]

Not at the moment, let me see what I can do tomorrow!

[justusdieckmann]

I've just had a look through the rest of the code, and I think it isn't really a problem with 4.1, but rather the ACL mode. I've pushed a commit and I believe the error should not occur anymore

[ferishili]

Hey @justusdieckmann,
thanks for the fix,
However, your fix now ends up providing a logical bug!

By "ACL Change" as for importmode and "Unchecked" as for importvideoscoredefaultvalue, the episodes are now getting the RED "X" no matter if you select the series to import or leave it unchecked, which is logically incorrect because it creates a big misunderstanding from UI/UX point of view.

IMHO they should always remain checked with green tick to avoid any confusion! The Help tooltip icon is also there to explain the logic!

Your opinions here are most welcome @mwuttke @justusdieckmann

BR

[mwuttke]

Hello @ferishili, Hello @justusdieckmann,

I've tested this pull request again by using the "ACL Change" and the "Unchecked" mode. And I think Farbod is right with his comment. We normally use "Duplicating Events". So I had forgotten to look at the other variant.

[justusdieckmann]

Hey,

ok, so I'm not completely sure if moodle even supports that kind of dependency, because it seems a dependency always causes the child to be disabled if the parent has a specific value (I couldn't find a dependency without any functional implication), and disabled settings that are turned on initially (or locked in the on state) throw an error.

Honestly, I just didn't want to fight the dependency setting anymore, so I've just listed the videos in the series setting in ACL mode. Does this work for you?

[mwuttke]

Hello @justusdieckmann.

It's not what I would expect in terms of look and feel, but it does what it's supposed to ;-)

Thanks a lot for your review!

And thanks a lot for you @ferishili for your work.

[ferishili]

Hi,
Thanks @justusdieckmann and @mwuttke,
From my side, that looks ok (it is not optimal, but it is acceptable), we could finally get this PR merged!

BR

[justusdieckmann]

Thank you very much!