search

catalyst / moodle-mod_subcourse

Subcourse module for Moodle
https://moodle.org/plugins/mod_subcourse
GNU General Public License v3.0
24 stars 41 forks source link

Moodle 27 stable #11

Closed vadimonus closed 9 years ago

vadimonus commented 10 years ago

Hi, David. I have recently found that your subcourse module is not yet updated to new log API, introduced in MOODLE 2.7. As far as i am intrested in using this plugin, i've fixed calls to deprecated add_to_log function. Also i have found that there is one submodule fork by Matt Gibson that has some extra features that are very useful and is needed for me and other members as discussed in russian comunity: ability to goto subcourse directly without visiting subcourse/view.php and ability to create course meta link in metacourse. This extra changes you can pull from branch master_David in https://github.com/vadimonus/moodle-mod_subcourse.git As far as you is owner of record in moodle plugins repository I'm asking you to include this changes to mainstream.

mudrd8mz commented 10 years ago

Thanks for your contribution Vadim. I am going to pick your commits into my repository asap. Regarding Matt's fork, I was aware of those additions. The feature of instantly jumping to the subcourse was just raised by another user couple of days ago and I am going to re-introduce it in the 2.7.0 release of the module. But, I was pretty sceptic regarding the auto-creating of the meta enrolment instance. IIRC, Matt's code did not take some edge cases into account and when I was attempting to fix that, I found that I was basically repeating a lot of logic that the enrolment management did. So I concluded that keeping things simple and stupid would be better approach here.

vadimonus commented 10 years ago

I have recently found that fetching grades with cron takes about a minute on my site with about 6000 subcource instatnces. So it's not a good idea to run it every 600 seconds on large sites. The way to make this time configurable is to convert cron to shedule tasks. If it can help you to introduce new version sooner, i can rewrite this part and send you pull request

mudrd8mz commented 10 years ago

Thanks for the offer. I have the migration to Task API in progress at my notebook already though.

vadimonus commented 10 years ago

Hello, David. What i have found recently: After upgrading to latest version it's possible to configure as cron as shedules tasks. This causes php notice: Notice: Undefined property: mod_subcourse\task\fetch_grades::$dayofmonth in C:\OpenServer\domains\localhost\el.kuzstu.ru\lib\classes\task\scheduled_task.php on line 343 This is not related to your module and is caused by missprint $this->dayofmonth instead of $this->day in referenced core code. Such error is not visible until you set shedule day of week to something other than *, like you have made in this module. I have not noticed any perfomance improvements in new version, but taking 1 minute for 10000 subcourses is enough to call it some times a day. To fully eliminate perfomance problem and any need of cron at all we can create event observer for \core\event\user_graded, which automatically fetches subcourse grade for refcourse from event. If doing this way, the intermediate page for subcourse with fetch button is not neeeded two, so all subcourses can work in instantredirect mode.

vadimonus commented 10 years ago

I've made event listeners that eliminates the need of cron. Please take a look at https://github.com/vadimonus/moodle-mod_subcourse master branch. Currently it's not very efficent, because it fethces grades of all users, while it can take grade for only one user

mudrd8mz commented 10 years ago

Yes, I am aware of the $dayofmonth issue and have submitted a patch fixing it in the core (MDL-45813). Thanks for the suggestion on the observer - it definitely makes sense. I'll look at it once the v2.7.0 of the plugin is out.

mudrd8mz commented 9 years ago

Conversion to the Task API included in the v2.7.0 release of the plugin. Thanks Vadim.