learnweb / moodle-tool_lifecycle

:recycle: Extensible Moodle plugin for managing course life cycles, e.g., deprovisioning
GNU General Public License v3.0
20 stars 32 forks source link

Use field courseid for logging events #203

Open melanietreitinger opened 6 months ago

melanietreitinger commented 6 months ago

Fixes #187

NinaHerrmann commented 6 months ago

Hey Melanie thank you for your contribution. Could you check why the tests are failing? :) Could you elaborate on why you think we should change the context? I do not have a very strong opinion on it but from my perspective, the process is still in the system context :thinking:

melanietreitinger commented 6 months ago

Hi Nina,

changing the context from 'system' to 'course' was necessary when using the log table field courseid- otherwise the tests failed with 'Inconsistent courseid - context combination detected'.

I fixed the tests - sorry for needing so many attempts... 🙈

justusdieckmann commented 6 months ago

Hey Melanie,

thank you very much! Nothing to feel sorry about :) I think maybe it would be nicer to not store the context in the process object, but instead create it in the event_from_process function just when needed. If I remember correctly, context objects are cached pretty heavily, so even instancing a context_course object once for every step in a workflow shouldn't cause much overhead

melanietreitinger commented 5 months ago

Hi @justusdieckmann,

I tried that in the first place, but it didn't work because when a course is deleted, the context is deleted, too, and it can't be re-created because the course is gone... 😢