search

xAPI-vle / moodle-logstore_xapi

A Moodle plugin to send xAPI statements to an LRS using events in the Moodle logstore.
GNU General Public License v3.0
75 stars 85 forks source link

'Include actions with these routes' setting does not work for \mod_book\event\chapter_viewed using cron #390

Closed garemoko closed 5 years ago

garemoko commented 5 years ago

Description I've been merrily testing using the new \mod_book\event\chapter_viewed event and it's working fine. I've only just realized that on my local Moodle it's been configured not to track so in fact it should not have been working. (I guess off is the default for new events).

image

Version https://github.com/xAPI-vle/moodle-logstore_xapi/pull/367/commits/08e72bdecc52fe26a4208e08a8dc82d169f8b80b on development

Steps to reproduce the bug

  1. Configure the plugin to use scheduled tasks with the \mod_book\event\chapter_viewed event disabled
  2. View a chapter in a book and confirm in the database that a \mod_book\event\chapter_viewed event has been triggered both in the standard and xapi logs
  3. Run cron.
  4. Confirm in the LRS that the \mod_book\event\chapter_viewed event has been sent

Expected behaviour The event should not be added to the xapi log table.

Actual behaviour The event is added to the table and is sent to the LRS.

ryasmi commented 5 years ago

That's really odd, yeah I think the default for new events is off which is pretty annoying. I don't know if there is a way to get around that.

In terms of it not actually disabling the event, I believe the is_event_ignored function is meant to handle that, but I guess it might be getting skipped in which case it would just inserting everything into the xapi logstore table on line 74.

BrendanHalley commented 5 years ago

I've just done some testing and the checkboxes for disabling statements are working as expected. Logs aren't entered into the xapi logstore table when backgroundmode is on and not sent at all when it's disables - as expected.

I'll install an older version of the plugin and "upgrade" it at some point to see if I can replicate the issue.

ryasmi commented 5 years ago

Thanks @BrendanHalley 👍

BrendanHalley commented 5 years ago

Just completed an "upgrade" test.

@garemoko @ryansmith94 I'd say this one can be closed, I'm unable to replicate.

ryasmi commented 5 years ago

Ok will close this and reopen if needed. Thank you for investigating @BrendanHalley, really appreciated 👍

garemoko commented 5 years ago

Weird. Thanks for testing.