StudentQuiz: Call require_login function for the StudentQuiz pages (#514

) Resolve the error missing param "userid" when accessing the StudentQuiz activity link without being logged in. Ensure that the require_login() function is called appropriately for the StudentQuiz pages. --------- Co-authored-by: hieuvu <[email protected]>
studentquiz · Dec 3, 2024 · 0406734 · 0406734
1 parent fb94f2b
commit 0406734
Show file tree

Hide file tree

Showing 19 changed files with 55 additions and 127 deletions.
diff --git a/attempt.php b/attempt.php
@@ -35,27 +35,13 @@
 // Comment highlight.
 $highlight = optional_param('highlight', 0, PARAM_INT);
 
-// Load course and course module requested.
-if ($cmid) {
-    if (!$cm = get_coursemodule_from_id('studentquiz', $cmid)) {
-        throw new moodle_exception("invalidcoursemodule");
-    }
-    if (!$course = $DB->get_record('course', array('id' => $cm->course))) {
-        throw new moodle_exception("coursemisconf");
-    }
-} else {
-    throw new moodle_exception("invalidcoursemodule");
-}
-
-// Authentication check.
-require_login($cm->course, false, $cm);
-
 $attemptid = required_param('id', PARAM_INT);
 $slot = required_param('slot', PARAM_INT);
 $returnurl = optional_param('returnurl', '', PARAM_LOCALURL);
 
+[$course, $cm] = get_course_and_cm_from_cmid($cmid, 'studentquiz');
+require_login($course, false, $cm);
 $attempt = $DB->get_record('studentquiz_attempt', array('id' => $attemptid));
-
 $context = context_module::instance($cm->id);
 
 // Check to see if any roles setup has been changed since we last synced the capabilities.

diff --git a/changestate.php b/changestate.php
@@ -36,8 +36,9 @@
 $returnurl = optional_param('returnurl', 0, PARAM_LOCALURL);
 $cmid = optional_param('cmid', 0, PARAM_INT);
 $courseid = optional_param('courseid', 0, PARAM_INT);
-$course = get_course($courseid);
-$cm = get_coursemodule_from_id('studentquiz', $cmid, $courseid, false, MUST_EXIST);
+
+[$course, $cm] = get_course_and_cm_from_cmid($cmid, 'studentquiz');
+require_login($course, false, $cm);
 $context = context_module::instance($cmid);
 require_capability('mod/studentquiz:changestate', $context);
 
@@ -54,19 +55,6 @@
 $PAGE->set_heading($course->fullname);
 $PAGE->activityheader->disable();
 $PAGE->set_secondary_active_tab("studentquiz");
-// Load course and course module requested.
-if ($cmid) {
-    if (!$module = get_coursemodule_from_id('studentquiz', $cmid)) {
-        throw new moodle_exception("invalidcoursemodule");
-    }
-    if (!$course = $DB->get_record('course', array('id' => $module->course))) {
-        throw new moodle_exception("coursemisconf");
-    }
-} else {
-    throw new moodle_exception("invalidcoursemodule");
-}
-
-require_login($module->course, false, $module);
 
 $rawquestionids = mod_studentquiz_helper_get_ids_by_raw_submit($_REQUEST);
 // If user has already confirmed the action.

diff --git a/classes/completion/custom_completion.php b/classes/completion/custom_completion.php
@@ -42,7 +42,8 @@ public function get_state(string $rule): int {
 
         $studentquizid = $this->cm->instance;
         $studentquiz = $DB->get_record('studentquiz', ['id' => $studentquizid], '*', MUST_EXIST);
-        $report = new mod_studentquiz_report($this->cm->id, $this->userid);
+        $course = get_course($studentquiz->course);
+        $report = new mod_studentquiz_report($course, $this->cm, $this->userid);
         $userstats = $report->get_user_stats();
 
         if (!$userstats) {

diff --git a/classes/local/studentquiz_question.php b/classes/local/studentquiz_question.php
@@ -36,7 +36,7 @@ class studentquiz_question {
     /** @var question_definition $question - Question class. */
     private $question;
 
-    /** @var stdClass $cm - Module. */
+    /** @var stdClass|\cm_info $cm - Module. */
     private $cm;
 
     /** @var \context_module  $context - Context. */
@@ -96,10 +96,11 @@ public function get_studentquiz(): stdClass {
 
     /**
      * Get course module.
+     * If we already have an existing cm, we will get from the class. Otherwise, get from database will return stdClass.
      *
-     * @return stdClass
+     * @return stdClass|\cm_info
      */
-    public function get_cm(): stdClass {
+    public function get_cm(): mixed {
         if (!isset($this->cm)) {
             $this->cm = get_coursemodule_from_id('studentquiz', $this->data->cmid);
         }

diff --git a/commenthistory.php b/commenthistory.php
@@ -33,22 +33,12 @@
 $studentquizquestionid = required_param('studentquizquestionid', PARAM_INT);
 $commentid = required_param('commentid', PARAM_INT);
 
-// Load course and course module requested.
-if ($cmid) {
-    $cm = get_coursemodule_from_id('studentquiz', $cmid);
-    if (!$cm) {
-        throw new moodle_exception("invalidcoursemodule");
-    }
-    if (!$comment = $DB->get_record('studentquiz_comment', ['id' => $commentid])) {
-        throw new moodle_exception("invalidcommentmodule");
-    }
-} else {
-    throw new moodle_exception("invalidcoursemodule");
+[$course, $cm] = get_course_and_cm_from_cmid($cmid, 'studentquiz');
+require_login($course, false, $cm);
+if (!$comment = $DB->get_record('studentquiz_comment', ['id' => $commentid])) {
+    throw new moodle_exception("invalidcommentmodule");
 }
 
-// Authentication check.
-require_login($cm->course, false, $cm);
-
 // Load context.
 $context = context_module::instance($cm->id);
 

diff --git a/hideaction.php b/hideaction.php
@@ -40,26 +40,15 @@
 $hide = required_param('hide', PARAM_INT);
 
 // Load course and course module requested.
-if ($cmid) {
-    if (!$module = get_coursemodule_from_id('studentquiz', $cmid)) {
-        throw new moodle_exception("invalidcoursemodule");
-    }
-    if (!$course = $DB->get_record('course', array('id' => $module->course))) {
-        throw new moodle_exception("coursemisconf");
-    }
-} else {
-    throw new moodle_exception("invalidcoursemodule");
-}
-
-// Authentication check.
-require_login($module->course, false, $module);
+[$course, $cm] = get_course_and_cm_from_cmid($cmid, 'studentquiz');
+require_login($course, false, $cm);
 require_sesskey();
 
-$studentquizquestion = mod_studentquiz_init_single_action_page($module, $studentquizquestionid);
+$studentquizquestion = mod_studentquiz_init_single_action_page($cm, $studentquizquestionid);
 
 $hidestatus = $hide ? \mod_studentquiz\local\studentquiz_helper::STATE_HIDE : \mod_studentquiz\local\studentquiz_helper::STATE_SHOW;
 $eventname = $hide ? 'hidden' : 'unhidden';
 $studentquizquestion->change_hidden_status($hide);
 $studentquizquestion->save_action($hidestatus, $USER->id);
-mod_studentquiz_event_notification_question($eventname, $studentquizquestion, $course, $module);
+mod_studentquiz_event_notification_question($eventname, $studentquizquestion, $course, $cm);
 redirect($returnurl);
diff --git a/pinaction.php b/pinaction.php
@@ -39,24 +39,12 @@
 $returnurl = required_param('returnurl', PARAM_LOCALURL);
 $pin = required_param('pin', PARAM_INT);
 
-// Load course and course module requested.
-if ($cmid) {
-    if (!$module = get_coursemodule_from_id('studentquiz', $cmid)) {
-        throw new moodle_exception("invalidcoursemodule");
-    }
-    if (!$course = $DB->get_record('course', array('id' => $module->course))) {
-        throw new moodle_exception("coursemisconf");
-    }
-} else {
-    throw new moodle_exception("invalidcoursemodule");
-}
-
-// Authentication check.
-require_login($module->course, false, $module);
+[$course, $cm] = get_course_and_cm_from_cmid($cmid, 'studentquiz');
+require_login($course, false, $cm);
 require_sesskey();
 
-$studentquizquestion = mod_studentquiz_init_single_action_page($module, $studentquizquestionid);
+$studentquizquestion = mod_studentquiz_init_single_action_page($cm, $studentquizquestionid);
 $eventname = $pin ? 'pinned' : 'unpinned';
 $studentquizquestion->change_pin_status($pin);
-mod_studentquiz_event_notification_question($eventname, $studentquizquestion, $course, $module);
+mod_studentquiz_event_notification_question($eventname, $studentquizquestion, $course, $cm);
 redirect($returnurl);
diff --git a/preview.php b/preview.php
@@ -32,21 +32,8 @@
 $cmid = required_param('cmid', PARAM_INT);
 $studentquizquestionid = required_param('studentquizquestionid', PARAM_INT);
 
-// Load course and course module requested.
-if ($cmid) {
-    if (!$module = get_coursemodule_from_id('studentquiz', $cmid)) {
-        throw new moodle_exception("invalidcoursemodule");
-    }
-    if (!$course = $DB->get_record('course', array('id' => $module->course))) {
-        throw new moodle_exception("coursemisconf");
-    }
-} else {
-    throw new moodle_exception("invalidcoursemodule");
-}
-
-// Authentication check.
-require_login($module->course, false, $module);
-
+[$course, $module] = get_course_and_cm_from_cmid($cmid, 'studentquiz');
+require_login($course, false, $module);
 // Load context.
 $context = context_module::instance($module->id);
 

diff --git a/reportlib.php b/reportlib.php
@@ -170,30 +170,34 @@ public function get_useractivitystats() {
 
     /**
      * Constructor assuming we already have the necessary data loaded.
-     * @param int|string $cmid course_module id
+     *
+     * @param object $course
+     * @param cm_info|object $cm course_module object
      * @param int|null $userid user id.
      */
-    public function __construct($cmid, ?int $userid = null) {
+    public function __construct(stdClass $course, object $cm, ?int $userid = null) {
         global $DB, $USER;
-        if (!$this->cm = get_coursemodule_from_id('studentquiz', $cmid)) {
+        if (!$cm) {
             throw new mod_studentquiz_view_exception($this, 'invalidcoursemodule');
         }
-        if (!$this->course = $DB->get_record('course', array('id' => $this->cm->course))) {
+        if (!$course) {
             throw new mod_studentquiz_view_exception($this, 'coursemisconf');
         }
 
         if (!$this->studentquiz = $DB->get_record('studentquiz',
-            array('coursemodule' => $this->cm->id, 'course' => $this->course->id))) {
+            ['coursemodule' => $cm->id, 'course' => $course->id])) {
             throw new mod_studentquiz_view_exception($this, 'studentquiznotfound');
         }
 
+        $this->course = $course;
+        $this->cm = $cm;
         $this->context = context_module::instance($this->cm->id);
 
         $this->userid = $USER->id;
         if ($userid) {
             $this->userid = $userid;
         }
-        $this->availablequestions = mod_studentquiz_count_questions($cmid);
+        $this->availablequestions = mod_studentquiz_count_questions($this->cm->id);
 
         \mod_studentquiz\utils::set_default_group($this->cm);
         $this->groupid = groups_get_activity_group($this->cm, true);

diff --git a/reportrank.php b/reportrank.php
@@ -31,9 +31,9 @@
     $cmid = required_param('cmid', PARAM_INT);
 }
 
-$report = new mod_studentquiz_report($cmid);
-$cm = $report->get_coursemodule();
-require_login($report->get_course(), false, $cm);
+[$course, $cm] = get_course_and_cm_from_cmid($cmid, 'studentquiz');
+require_login($course, false, $cm);
+$report = new mod_studentquiz_report($course, $cm);
 $context = $report->get_context();
 $output = $PAGE->get_renderer('mod_studentquiz', 'ranking');
 
@@ -51,4 +51,4 @@
 echo $OUTPUT->footer();
 
 // Trigger report rank viewed event.
-mod_studentquiz_reportrank_viewed($report->get_cm_id(), $context);
+mod_studentquiz_reportrank_viewed($cmid, $context);
diff --git a/reportstat.php b/reportstat.php
@@ -31,11 +31,10 @@
     $cmid = required_param('cmid', PARAM_INT);
 }
 
-$report = new mod_studentquiz_report($cmid);
-$cm = $report->get_coursemodule();
-
-require_login($report->get_course(), false, $cm);
-$context = context_module::instance($cmid);
+[$course, $cm] = get_course_and_cm_from_cmid($cmid, 'studentquiz');
+require_login($course, false, $cm);
+$report = new mod_studentquiz_report($course, $cm);
+$context = $report->get_context();
 $renderer = $PAGE->get_renderer('mod_studentquiz', 'report');
 
 $PAGE->set_title($report->get_statistic_title());

diff --git a/save.php b/save.php
@@ -33,16 +33,14 @@
 require_once(__DIR__ . '/../../config.php');
 require_once(__DIR__ . '/locallib.php');
 
-global $COURSE;
+use mod_studentquiz\utils;
 
 // Get parameters.
 $cmid = required_param('cmid', PARAM_INT);
 $studentquizquestionid = required_param('studentquizquestionid', PARAM_INT);
 $save = required_param('save', PARAM_NOTAGS);
 
 [$course, $cm] = get_course_and_cm_from_cmid($cmid, 'studentquiz');
-
-// Authentication check.
 require_login($course, false, $cm);
 require_sesskey();
 

diff --git a/tests/backup_test.php b/tests/backup_test.php
@@ -208,10 +208,11 @@ public function test_old_sq_backup_data(string $filename, string $coursefullname
         // Check question with question name is not exist before restore.
         $this->assertFalse($DB->record_exists('question', ['name' => $questionname]));
         $sq = $this->restore_sq_backup_file_to_course_shortname($filename, $coursefullname, $courseshortname);
+        [$course, $cm] = get_course_and_cm_from_cmid($sq->coursemodule, 'studentquiz');
         // Check question with question name exist after restore.
         $this->assertTrue($DB->record_exists('question', ['name' => $questionname]));
 
-        $report = new \mod_studentquiz_report($sq->coursemodule);
+        $report = new \mod_studentquiz_report($course, $cm);
         $count = 0;
         // Check ranking page.
         foreach ($report->get_user_ranking_table() as $ur) {

diff --git a/tests/bank_performance_test.php b/tests/bank_performance_test.php
@@ -84,7 +84,7 @@ public function run_questionbank($result) {
             'showallprinted' => 0,
         );
 
-        $report = new mod_studentquiz_report($result['cm']->id);
+        $report = new mod_studentquiz_report($result['course'], $result['cm']);
         if (utils::moodle_version_is("<=", "42")) {
             $questionbank = new studentquiz_bank_view_pre_43(
                 new \core_question\local\bank\question_edit_contexts(\context_module::instance($result['cm']->id)),

diff --git a/tests/bank_view_test.php b/tests/bank_view_test.php
@@ -112,7 +112,7 @@ public function run_questionbank() {
                 'qpage' => 0,
         );
 
-        $report = new \mod_studentquiz_report($this->cm->id);
+        $report = new \mod_studentquiz_report($this->course, $this->cm);
         if (utils::moodle_version_is("<=", "42")) {
             $questionbank = new studentquiz_bank_view_pre_43(
                 new \core_question\local\bank\question_edit_contexts(\context_module::instance($this->cm->id)),

diff --git a/tests/report_test.php b/tests/report_test.php
@@ -87,7 +87,7 @@ protected function setUp(): void {
         $this->context = \context_module::instance($activity->cmid);
         $this->studentquiz = mod_studentquiz_load_studentquiz($activity->cmid, $this->context->id);
         $this->cm = get_coursemodule_from_id('studentquiz', $activity->cmid);
-        $this->report = new \mod_studentquiz_report($activity->cmid);
+        $this->report = new \mod_studentquiz_report($course, $this->cm);
 
         // Create users.
         $usernames = array('Peter', 'Lisa', 'Sandra', 'Tobias', 'Gabi', 'Sepp');

diff --git a/tests/viewlib_test.php b/tests/viewlib_test.php
@@ -60,7 +60,7 @@ protected function setUp(): void {
         $_GET['cmid'] = $this->cm->id;
 
         // Satisfy codechecker: $course $cm $studentquiz $userid.
-        $report = new \mod_studentquiz_report($this->cm->id);
+        $report = new \mod_studentquiz_report($course, $this->cm);
         $this->viewlib = new \mod_studentquiz_view($course, $context, $this->cm,
             $studentquiz, $user->id, $report);
     }

diff --git a/view.php b/view.php
@@ -37,15 +37,11 @@
     // but moodle allows to view a mod page with parameter id in place of cmid.
     $_GET['cmid'] = $cmid;
 }
-
-// TODO: make course-, context- and login-check in a better starting class (not magically hidden in "report").
-// And when doing that, offer course, context and studentquiz object over it, which all following actions can use.
-$report = new mod_studentquiz_report($cmid);
-require_login($report->get_course(), false, $report->get_coursemodule());
-
-$course = $report->get_course();
+[$module, $cm] = get_module_from_cmid($cmid);
+$course = get_course($cm->course);
+require_login($course, false, $cm);
+$report = new mod_studentquiz_report($course, $cm);
 $context = $report->get_context();
-$cm = $report->get_coursemodule();
 
 $studentquiz = mod_studentquiz_load_studentquiz($cmid, $context->id);
 

diff --git a/viewlib.php b/viewlib.php
@@ -129,7 +129,7 @@ private function load_questionbank() {
         $params = $_GET;
         // Get edit question link setup.
         list($thispageurl, $contexts, $cmid, $cm, $module, $pagevars)
-            = question_edit_setup('questions', '/mod/studentquiz/view.php', true);
+            = question_edit_setup('questions', '/mod/studentquiz/view.php');
         $pagevars['qperpage'] = optional_param('qperpage', \mod_studentquiz\utils::DEFAULT_QUESTIONS_PER_PAGE, PARAM_INT);
         $pagevars['showall'] = optional_param('showall', false, PARAM_BOOL);
         $pagevars['cat'] = $this->get_category_id() . ',' . $this->get_context_id();