Keep seriesmapping records when deleting a block (configurable)

ferishili commented 7 months ago

This PR fixes #351

Description

please refer to the related issue #351

Important to know

The logical way (current way) is to remove any related records upon deleting a block, therefore you do not need to worry about the rest of cleaning and everything else. With that said, the current way of removing seresmapping record upon deleting a block is correct but not enough.
This feature request will leave orphan records in the database table (which is a NO-GO), that results in a huge and messy table in a long-run that compromises the performance. Therefore, it is designed to be configurable that brings attention to its cons!

How it works

In plugin settings under General settings > Settings for a block instance a new setting option is introduced Keep serie mapping after deleting the block which toggles this feature.
When activated, it refuses to delete the course-series mapping record from the related table, otherwise, it goes on like before.

How to test

Make sure you have the latest plugin versions.
Have this PR patched.
Head over to plugin admin setting and enable the above mentioned setting option.
In a test course with edit mode on:
- Make sure Opencast Block is added and has (content) a series and videos in it.
- Delete the Opencast Block.
- Add the Opencast Block again.
- Confirm that the series and videos are there and nothing is missing!
To test the deactivated scenario, just repeat the above steps with the setting off, but make sure your videos and series in that block are temporary and are for the sake of this test!

What needs to be done in near future

The proper way to do is to have a back-up table that records the deleted courses and series mapping and get the record back when re-adding the block. In this way, you have a separate table and the performance won't be compromised.

NOTE: behat test scenario is included

NinaHerrmann commented 7 months ago

Hey @ferishili, Thank you for your contribution! I am not completely convinced about the design of the feature. I would prefer to have a warning when deleting the block. @justusdieckmann any opinion on this?

ferishili commented 7 months ago

Hey @ferishili, Thank you for your contribution! I am not completely convinced about the design of the feature. I would prefer to have a warning when deleting the block. @justusdieckmann any opinion on this?

Hi @NinaHerrmann, In fact there is a confirmation, before deleting the block, which says are you sure to delete the block and one has to actively select if would liek to delete or not. However, the test of that confirmation is coming from Moodle itself and it is safe to say it is not much we can change!

NinaHerrmann commented 7 months ago

Even better would be a warning which enables the user to choose between deleting the videos, canceling, and keeping the videos in opencast.

NinaHerrmann commented 7 months ago

@justusdieckmann could you please check if there is no option to change the moodle standard message when deleting blocks?

ferishili commented 7 months ago

Even better would be a warning which enables the user to choose between deleting the videos, canceling, and keeping the videos in opencast.

I am not sure if Moodle offers such a thing! And please keep in mind that we are not in Opencast block anymore, and this deletion is happening on the course - block level

If that is possible that would be an ultimate solution to that 👍

ferishili commented 7 months ago

And BTW the failed behats are excatly because of different ways of displaying that confirmation dialogue in moodle < 4.0

Are we still supporting moodle < 4.0? Is there a plan to discontinue the support of moodle 3.x versions?

NinaHerrmann commented 7 months ago

By the way I think has behat is a very nice label :joy:

NinaHerrmann commented 7 months ago

I don't see a point in testing everything that will be outdated on 13.12. I think you have the permissions to remove it from the ci-script right? It would be helpful if you could change that! :)

ferishili commented 7 months ago

I will be checking and get that done tonight. In my opinion, it is a good practice to announce it to the community and also remove that support from everywhere else like plugin version etc. (of course for the next release)

NinaHerrmann commented 7 months ago

Good point! I agree - and we can definitely announce it at the next Meeting :+1:

ferishili commented 7 months ago

Hi @NinaHerrmann, the support for Moodle 3.9 - 4.0 is now lifted, and block_opencast as well as tool_opencast plugin versions are updated, I left the rest of them to you to decide if they would get this versioning or not! BR

NinaHerrmann commented 7 months ago

Thanks for lifting the support - personally I am not a fan of committing changes to the master without changing anything but support - but to be honest it doesn't harm anyone. Justus Dieckmann is currently unavailable so the pull request will be checked as soon as he is available :) Thanks again - also for behat testing! :heart_eyes:

ferishili commented 6 months ago

Even better would be a warning which enables the user to choose between deleting the videos, canceling, and keeping the videos in opencast.

Hi @NinaHerrmann, @justusdieckmann, To avoid double-work, just wanted to inform you that, I started looking for a way to provide a customized block delete modal/page. Please let me know if you have started this. Otherwise, I will take over.

Thanks

NinaHerrmann commented 6 months ago

Hey @ferishili we already took a look at it and would not recommend it. We are waiting for @justusdieckmann to recover to test the pull request and merge it. Thanks for your patience!

ferishili commented 6 months ago

I am seeing some possibility to make it work the way we want, so, please give me a few days to provide a proof of concept.

ferishili commented 6 months ago

Also, your findings and justification would help me a lot. I would be very happy to have a short meeting to discuss this.

Thanks in advance

NinaHerrmann commented 6 months ago

Sure, we found a way to have an additional page after submitting the first Pop-up. However, it is not standard Moodle behavior, which is not my favorite implementation. Is that the same proof of concept you are planning? Any other way seems to adjust moodle behavior in a weird unexpected way - e.g. changing text with Javascript is not an option for me.

ferishili commented 6 months ago

The way I managed to find seems to be a standard way, let make a draft and then I will propose

NinaHerrmann commented 6 months ago

Thank you very much for your work, a standard way would be amazing :smiling_face_with_three_hearts:

ferishili commented 6 months ago

Hi @NinaHerrmann, hi @mwuttke, Please have a look at #354 & #355 with video tutorials. (of course, I will make all the test green :D)

Thanks in advance

ferishili commented 4 months ago

Close as completed in https://github.com/Opencast-Moodle/moodle-block_opencast/pull/355

Opencast-Moodle / moodle-block_opencast