Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

$event->id does not exist in multiple places. #234

Open
caperneoignis opened this issue Aug 22, 2018 · 5 comments
Open

$event->id does not exist in multiple places. #234

caperneoignis opened this issue Aug 22, 2018 · 5 comments
Labels

Comments

@caperneoignis
Copy link
Contributor

caperneoignis commented Aug 22, 2018

Description
When going through a quiz, with the logstore app set to real time transmission to the LRS. I get the following in several places.

Notice: Undefined property: stdClass::$id in {web_directory}\moodle\admin\tool\log\store\xapi\src\transformer\events\mod_quiz\attempt_viewed.php on line 37

And again at transformer\handler.php line 37

$eventobj->id is undefined.
Version

  • master, version 2018073005

Steps to reproduce the bug

  1. Have debugging turned on for moodle.
    Go and take a quiz, and look at the bottom, you will see several undefined warnings.

Expected behaviour

  • No warnings, because the id has either been set, or there is a check to replace the value with the expected value, or ignore the value all together.

Actual behaviour

  • The id is not being set in the stdClass causing errors and in some cases bad values/no event sent.
@caperneoignis
Copy link
Contributor Author

We can not assume event will have an id, we need to handle this/assign this appropriately.

@ryasmi ryasmi added the fix label Aug 22, 2018
@ryasmi
Copy link
Member

ryasmi commented Aug 22, 2018

Yeah I'd recommend checking if it's there and setting it to null if not.

@caperneoignis
Copy link
Contributor Author

caperneoignis commented Aug 22, 2018

Any opposition to using a ternary expression in those spots? And to use zero instead of null since null can put us back into the undefined boat depending on where that element ends up?

@ryasmi
Copy link
Member

ryasmi commented Aug 23, 2018

I can see four uses of the event's id.

  1. /src/transformer/events/mod_quiz/attempt_viewed.php#L37
  2. /src/loader/lrs.php#L70
  3. /src/transformer/handler.php#L46
  4. /classes/task/emit_task.php#L46

Let me know if there are any other uses. I think the first usage is actually meant to be $event->objectid rather than the $event->id. If that's the case, we should probably just avoid using the event id, except where it's absolutely needed like the 4th usage where the id will have to exist.

@caperneoignis
Copy link
Contributor Author

That's agreeable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants