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

Undefined offset: 0 #167

Closed
ryansmith94 opened this issue Mar 26, 2018 · 12 comments

Comments

@ryansmith94
Copy link
Member

commented Mar 26, 2018

Message from Frank Troglauer on the Moodle plugin directory.

Just a heads up that in debugging mode we are getting a notice from your plugin of an undefined offset when using Moodles bulk upload via csv feature.

Notice: Undefined offset: 0 in D:\WWP\wamp\www\NGT2\lms\admin\tool\log\store\xapi\classes\log\store.php on line 158

Might changing line 157 of store.php to this line of code remove the notice while not causing any issues with your plugin?
if (is_numeric($key) && isset($xapievents[$key]['context']['extensions'][Event::CONTEXT_EXT_KEY]['id'])) {

@caperneoignis

This comment has been minimized.

Copy link
Contributor

commented Apr 20, 2018

I was just about to start this issue my self until I saw your issue.

This might be solvable by adding a isset. But I was also seeing this happen in Moodle itself if I am logged in as an admin who is assigned to a course. I believe since CLI is also ran as admin there is a chance this is related to the user type? But I'm spit balling here.

I am using:
Plugin version 2017111300
Moodle version 3.4.1+
php version 7.0 and 7.1
I have not tried to determine if this also happens for a non-priv user and in what cases, if it depends on module or if it happens in course context also.

@caperneoignis

This comment has been minimized.

Copy link
Contributor

commented Apr 20, 2018

if I have time, I will try debugging and see the exact moment when this occurs and in what instance so we can make a more percise fix. However, I'm pretty floored with other work, so I'm not sure if I can hit this or not.

@ryansmith94

This comment has been minimized.

Copy link
Member Author

commented Apr 20, 2018

Hey, thanks for the input Lee 👍 Also pretty floored with other work at the moment.

@davidpesce

This comment has been minimized.

Copy link
Contributor

commented May 2, 2018

Confirming this is an issue. I'm getting it with developer level errors turned on with the default Moodle home page. Haven't dug too far past that.

@davidpesce

This comment has been minimized.

Copy link
Contributor

commented May 2, 2018

Looks like it happens on course and quiz views as well

@caperneoignis

This comment has been minimized.

Copy link
Contributor

commented Jun 27, 2018

I have figured it out. What is going on is this.

line 158 on store.php in classes\log\ directory has this line: $k = $xapievents[$key]['context']['extensions'][Event::CONTEXT_EXT_KEY]['id'];

However, the array looks like this:

array (
  'statements' => 
  array (
    0 => 
    array (
      'actor' => 
      array (
        'name' => 'Admin User',
        'mbox' => 'test@example.com',
      ),
      'context' => 
      array (
        'platform' => 'Moodle',
        'language' => 'en',
        'extensions' => 
        array (
          'http://lrs.learninglocker.net/define/extensions/moodle_logstore_standard_log' => 
          array (
            'eventname' => '\\mod_quiz\\event\\attempt_reviewed',
            'component' => 'mod_quiz',
            'action' => 'reviewed',
            'target' => 'attempt',
            'objecttable' => 'quiz_attempts',
            'objectid' => '5',
            'crud' => 'r',
            'edulevel' => 1,
            'contextid' => 40,
            'contextlevel' => 70,
            'contextinstanceid' => '4',
            'userid' => '2',
            'courseid' => '2',
            'relateduserid' => '2',
.....

NOTE: I shorted the array for read ability because it was huge before.

The very top part is the important part, the 'statements' piece. As you can see, the $key int value is incorrect, not to mention the context element is still another level deep. Not sure where this call is, or if we need to add a function or a call to pop out the statements piece so we can read the array like normal. This does not appear unless you have multiple statements being sent as once. IE quiz submitted, question answered is when I saw this.

@caperneoignis

This comment has been minimized.

Copy link
Contributor

commented Jun 27, 2018

Looks like, if we change $xapievents to $statements this will resolve the issue. In fact looks like that should be what was done but was missed? because that would solve this issue, especially since we are taking the count of $statements before running the foreach loop.

@caperneoignis

This comment has been minimized.

Copy link
Contributor

commented Jun 27, 2018

So tried that fix, and now we have a new issue. 'id' is not in that extension array. Not sure which 'id' you are looking for there.

http://lrs.learninglocker.net/define/extensions/moodle_logstore_standard_log => array (
  'eventname' => '\\mod_quiz\\event\\course_module_viewed',
  'component' => 'mod_quiz',
  'action' => 'viewed',
  'target' => 'course_module',
  'objecttable' => 'quiz',
  'objectid' => '1',
  'crud' => 'r',
  'edulevel' => 2,
  'contextid' => 40,
  'contextlevel' => 70,
  'contextinstanceid' => '4',
  'userid' => '2',
  'courseid' => '2',
  'relateduserid' => NULL,
  'anonymous' => 0,
  'other' => 'N;',
  'timecreated' => 1530114571,
  'origin' => 'web',
  'ip' => '10.168.2.54',
  'realuserid' => NULL,
)
@caperneoignis

This comment has been minimized.

Copy link
Contributor

commented Jun 27, 2018

Oh, I'm also on 3.5 now, just in case 'id' exist in other implementations.

@garemoko

This comment has been minimized.

Copy link
Contributor

commented Jun 27, 2018

The id should be the value of the id column of the logstore_xapi_log table and it's used to delete the event from the table once it's sent. See https://github.com/xAPI-vle/moodle-logstore_xapi/blob/master/classes/task/emit_task.php#L53-L58

So the question is why is the id not in there, or possibly why are we looking there for the id in the first place?

@jjs105

This comment has been minimized.

Copy link

commented Jul 3, 2018

I too have stumbled onto this (these) issues in the last couple of days.

First up to address looking for the ID in the wrong place I suggest the following change ...

foreach (array_keys($statements) as $key) {
    if (is_numeric($key)) {
        $k = $xapievents[$key]['context']['extensions'][Event::CONTEXT_EXT_KEY]['id'];
        $sentevents[$k] = $this->getlast_action_result($response);
    }
}

to ..

foreach ($statements as $statement) {
    if (isset($statement['context']['extensions'][Event::CONTEXT_EXT_KEY]['id'])) {
        $k = $statement['context']['extensions'][Event::CONTEXT_EXT_KEY]['id'];
        $sentevents[$k] = $this->getlast_action_result($response);
    }
}

The second issue of the ID key being available - this is dependent on the batch mode setting. If not in batch mode then the ID isn't available as the event(s) are being processed by the xapi store without any knowledge of the DB table.

When in batch mode however the events data has been created from the DB table and both is indexed by the ID and contains the ID in the extensions data.

@ryansmith94

This comment has been minimized.

@ryansmith94 ryansmith94 closed this Jul 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.