ewallah / moodle-availability_relativedate

Relative dates in Moodle
https://moodle.org/plugins/availability_relativedate
GNU General Public License v3.0
8 stars 7 forks source link

[v4.2.1] Bug (suspected, unconfirmed) - After disabling completion for activity A, activity B depending on A should automatically update #28

Open danowar2k opened 8 months ago

danowar2k commented 8 months ago

Scenario: You have two activities A and B, both have completion enabled at the beginning. You let B be available depending on completion of A and save. You then disable completion for activity A.

IS: B still depends on completion of A even if A can now never be completed. SHOULD: After update of A, B should be updated to remove the dependency on A (just like autoupdate does when I would delete A).

This is yet unconfirmed, but looks mighty likely to me that this is currently the case.

EDIT: It would probably be nice for users if you could generate a Moodle notification when such availability/completion changes lead to changed/broken availability conditions.

ewallah commented 8 months ago

Just some ideas:

danowar2k commented 8 months ago
  • For a long time I tried to figure out a way to generate a Moodle notification if someone deleted a module that was part of the course completion (making it impossible to students to ever complete their course). I never found a good solution. With the arrival of hooks in Moodle, perhaps this will become easier to implement...

https://docs.moodle.org/dev/Notifications

Couldn't you just (after you detect a deletion of or a disabled completion of a relevant course module) add a \core\notification::warning($message); to display on the next page load during the autoupdate?

In course, when you display the condition text under the activities, couldn't you generate a warning displayed to the teacher that the condition currently cannot be fulfilled because the course module it's depending on is unavailable/cannot be found?

danowar2k commented 8 months ago
  • What happens or should happen if you re enable the completion for activity A?

If completion for A is disabled, the condition should be removed and a notification should be shown that that condition had to be removed. If completion of A is then enabled again, nothing else should happen. The user should re-add the condition again by himself if he wants it. My opinion, of course. You can't help everyone and if you let the condition be stored in the database, it is, again in my opinion, "data which is lying".

EDIT: Of course your condition data could have another value member variable, let's call it x, where once completion for A is disabled the course module id of A is stored as long as it is disabled. That way you could keep m clean with -1 (or whatever) and could reactivate the condition if completion of A is reactivated. So you could say "as long as there is something in x this condition is always true.

danowar2k commented 8 months ago

I just looked at the code for availability_completion from core, see: https://github.com/moodle/moodle/blob/e567c21d6edb902e652fcbf1aaf4a49a82579f84/availability/condition/completion/classes/condition.php#L135-L182

Stuff to note:

If the completion data remains in the table, the completion check would still be doable. Which would make sense, because a user would still have completed the activity even if it isn't tracked anymore. But then it would be bad for users which wouldn't have the activity completed before it was disabled. So you basically would have to say "if completion of A is disabled, B must always return true for that condition". If you'd have it always be false, you couldn't have B available anymore.

ewallah commented 8 months ago

I think a teacher should be warned before deleting a module that is part of completion settings. It is to late to do so after the module is deleted.

In one instance I added this to core course/mod.php:

        if (!empty($CFG->enableavailability)) {
            if (\core_availability\info::completion_value_used($course, $cm->id)) {
                \core\notification::warning('This module is used in the module completion criteria of another module. If you delete this module, the other module will never be available.');
            }
            $params = ['course' => $cm->course, 'criteriatype' => COMPLETION_CRITERIA_TYPE_ACTIVITY, 'moduleinstance' => $cm->id];
            if ($DB->record_exists('course_completion_criteria', $params)) {
                \core\notification::error('This module is part of the course completion criteria. If you delete this module, the students will never be able to complete the course.');
            }
        }

Not the perfect solution, but a valid try. Perhaps it should be impossible to delete a module when it is used elsewhere?

ewallah commented 8 months ago

I created MDL-81149.