MDL-41185 cleanup course viewed events

Thanks Marina for spotting some of the issues.

course/user.php

@@ -105,12 +105,11 @@
     $mode = reset($modes);
 }
 
-$eventdata = array();
-$eventdata['courseid'] = $id;
-$eventdata['context'] = $coursecontext;
-$eventdata['userid'] = $user->id;
-$eventdata['other'] = array();
-$eventdata['other']['mode'] = $mode;
+$eventdata = array(
+    'context' => $coursecontext,
+    'relateduserid' => $user->id,
+    'other' => array('mode' => $mode),
+);
 $event = \core\event\course_user_report_viewed::create($eventdata);
 $event->trigger();
 

course/view.php

@@ -282,15 +282,11 @@
     echo html_writer::end_tag('div');
 
     // Trigger course viewed event.
-    $eventparams = array();
-    $eventparams['context'] = $context;
-    $eventparams['courseid'] = $course->id;
-    $eventparams['userid'] = $USER->id;
+    $eventdata = array('context' => $context);
     if (!empty($section)) {
-        $eventparams['other'] = array();
-        $eventparams['other']['coursesectionid'] = $section;
+        $eventdata['other'] = array('coursesectionid' => $section);
     }
-    $event = \core\event\course_viewed::create($eventparams);
+    $event = \core\event\course_viewed::create($eventdata);
     $event->trigger();
 
     // Include course AJAX

index.php

@@ -75,11 +75,9 @@
         }
     }
 
-    if (isloggedin()) {
-        $eventparams = array('courseid' => SITEID, 'context' => context_course::instance(SITEID));
-        $event = \core\event\course_viewed::create($eventparams);
-        $event->trigger();
-    }
+    $eventparams = array('context' => context_course::instance(SITEID));
+    $event = \core\event\course_viewed::create($eventparams);
+    $event->trigger();
 
 /// If the hub plugin is installed then we let it take over the homepage here
     if (file_exists($CFG->dirroot.'/local/hub/lib.php') and get_config('local_hub', 'hubenabled')) {

lib/classes/event/course_user_report_viewed.php

@@ -42,8 +42,6 @@ class course_user_report_viewed extends \core\event\base {
     /**
      * Init method.
      *
-     * Please override this in extending class and specify objecttable.
-     *
      * @return void
      */
     protected function init() {
@@ -57,7 +55,7 @@ protected function init() {
      * @return string
      */
     public function get_description() {
-        return "A user with the id '$this->userid' viewed the user report in the course  '$this->courseid'";
+        return "A user with the id '$this->userid' viewed the user report in the course '$this->courseid' for user '$this->relateduserid'";
     }
 
     /**
@@ -75,7 +73,7 @@ public static function get_name() {
      * @return \moodle_url
      */
     public function get_url() {
-        return new \moodle_url("/course/user.php", array('id' => $this->courseid, 'user' => $this->userid,
+        return new \moodle_url("/course/user.php", array('id' => $this->courseid, 'user' => $this->relateduserid,
                 'mode' => $this->other['mode']));
     }
 
@@ -86,7 +84,7 @@ public function get_url() {
      */
     protected function get_legacy_logdata() {
         return array($this->courseid, 'course', 'user report', 'user.php?id=' . $this->courseid . '&user='
-                . $this->userid . '&mode=' . $this->other['mode'], $this->userid);
+                . $this->relateduserid . '&mode=' . $this->other['mode'], $this->relateduserid);
     }
 
     /**
@@ -96,6 +94,16 @@ protected function get_legacy_logdata() {
      * @return void
      */
     protected function validate_data() {
+        parent::validate_data();
+
+        if ($this->contextlevel != CONTEXT_COURSE) {
+            throw new \coding_exception('Context passed must be course context.');
+        }
+
+        if (empty($this->relateduserid)) {
+            throw new \coding_exception('relateduserid needs to be set.');
+        }
+
         // Make sure this class is never used without proper object details.
         if (!isset($this->other['mode'])) {
             throw new \coding_exception('mode needs to be set in $other.');

lib/classes/event/course_viewed.php

@@ -42,8 +42,6 @@ class course_viewed extends \core\event\base {
     /**
      * Init method.
      *
-     * Please override this in extending class and specify objecttable.
-     *
      * @return void
      */
     protected function init() {
@@ -57,7 +55,7 @@ protected function init() {
      * @return string
      */
     public function get_description() {
-        return "A user with the id '$this->userid' viewed the course  '$this->courseid'";
+        return "A user with the id '$this->userid' viewed the course '$this->courseid'";
     }
 
     /**
@@ -75,7 +73,11 @@ public static function get_name() {
      * @return \moodle_url
      */
     public function get_url() {
-        return \course_get_url($this->courseid, $this->other['coursesectionid']);
+        $sectionid = null;
+        if (isset($this->other['coursesectionid'])) {
+            $sectionid = $this->other['coursesectionid'];
+        }
+        return \course_get_url($this->courseid, $sectionid);
     }
 
     /**
@@ -84,12 +86,16 @@ public function get_url() {
      * @return array|null
      */
     protected function get_legacy_logdata() {
-        if (!empty($this->other['coursesectionid'])) {
+        if ($this->courseid == SITEID and !isloggedin()) {
+            // We did not log frontpage access in older Moodle versions.
+            return null;
+        }
+
+        if (isset($this->other['coursesectionid'])) {
             return array($this->courseid, 'course', 'view section', 'view.php?id=' . $this->courseid . '&sectionid='
                     . $this->other['coursesectionid'], $this->other['coursesectionid']);
-        } else {
-            return array($this->courseid, 'course', 'view', 'view.php?id=' . $this->courseid, $this->courseid);
         }
+        return array($this->courseid, 'course', 'view', 'view.php?id=' . $this->courseid, $this->courseid);
     }
 
     /**
@@ -99,6 +105,8 @@ protected function get_legacy_logdata() {
      * @return void
      */
     protected function validate_data() {
+        parent::validate_data();
+
         if ($this->contextlevel != CONTEXT_COURSE) {
             throw new \coding_exception('Context passed must be course context.');
         }

lib/tests/events_test.php

@@ -189,9 +189,8 @@ public function test_course_user_report_viewed() {
         $context = context_course::instance($course->id);
 
         $eventparams = array();
-        $eventparams['courseid'] = $course->id;
         $eventparams['context'] = $context;
-        $eventparams['userid'] = $user->id;
+        $eventparams['relateduserid'] = $user->id;
         $eventparams['other'] = array();
         $eventparams['other']['mode'] = 'grade';
         $event = \core\event\course_user_report_viewed::create($eventparams);
@@ -221,9 +220,7 @@ public function test_course_viewed() {
 
         // First try with no optional parameters.
         $eventparams = array();
-        $eventparams['courseid'] = $course->id;
         $eventparams['context'] = $context;
-        $eventparams['userid'] = $user->id;
         $event = \core\event\course_viewed::create($eventparams);
 
         // Trigger and capture the event.
@@ -239,13 +236,10 @@ public function test_course_viewed() {
         $this->assertEventContextNotUsed($event);
 
         // Now try with optional parameters.
+        $sectionid = 34;
         $eventparams = array();
-        $eventparams['courseid'] = $course->id;
         $eventparams['context'] = $context;
-        $eventparams['userid'] = $user->id;
-        $eventparams['other'] = array();
-        $sectionid = 34;
-        $eventparams['other']['coursesectionid'] = $sectionid;
+        $eventparams['other'] = array('coursesectionid' => $sectionid);
         $event = \core\event\course_viewed::create($eventparams);
 
         // Trigger and capture the event.