reduce php mess in ratings class

Author: TamaroWalter

classes/ratings.php

@@ -42,25 +42,18 @@ class ratings {
      * @param int    $postid
      * @param int    $rating
      * @param object $cm
-     * @param null   $userid
+     * @param int   $userid
      *
      * @return bool|int
      */
-    public static function moodleoverflow_add_rating($moodleoverflow, $postid, $rating, $cm, $userid = null) {
-        global $DB, $USER, $SESSION;
-
-        // Has a user been submitted?
-        if (!isset($userid)) {
-            $userid = $USER->id;
-        }
+    public static function moodleoverflow_add_rating($moodleoverflow, $postid, $rating, $cm, $userid) {
+        global $DB;
 
         // Is the submitted rating valid?
         $possibleratings = [RATING_NEUTRAL, RATING_DOWNVOTE, RATING_UPVOTE, RATING_SOLVED,
             RATING_HELPFUL, RATING_REMOVE_DOWNVOTE, RATING_REMOVE_UPVOTE,
             RATING_REMOVE_SOLVED, RATING_REMOVE_HELPFUL, ];
-        if (!in_array($rating, $possibleratings)) {
-            throw new moodle_exception('invalidratingid', 'moodleoverflow');
-        }
+        moodleoverflow_throw_exception_with_check(!in_array($rating, $possibleratings), 'invalidratingid');
 
         // Get the related post.
         $post = moodleoverflow_get_record_or_exception('moodleoverflow_posts', ['id' => $postid], 'invalidparentpostid');
@@ -85,26 +78,17 @@ public static function moodleoverflow_add_rating($moodleoverflow, $postid, $rati
         if (!self::moodleoverflow_user_can_rate($post, $modulecontext, $userid)) {
 
             // Catch unenrolled users.
-            if (!isguestuser() && !is_enrolled($coursecontext)) {
-                $SESSION->wantsurl = qualified_me();
-                $SESSION->enrolcancel = get_local_referer(false);
-                redirect(new \moodle_url('/enrol/index.php', [
-                    'id' => $course->id,
-                    'returnurl' => '/mod/moodleoverflow/view.php?m' . $moodleoverflow->id,
-                ]), get_string('youneedtoenrol'));
-            }
+            $returnurl = '/mod/moodleoverflow/view.php?m' . $moodleoverflow->id;
+            moodleoverflow_catch_unenrolled_user($coursecontext, $course->id, $returnurl);
 
             // Notify the user, that he can not post a new discussion.
             throw new moodle_exception('noratemoodleoverflow', 'moodleoverflow');
         }
 
         // Make sure post author != current user, unless they have permission.
-        if (($post->userid == $userid) && !
-            (($rating == RATING_SOLVED || $rating == RATING_REMOVE_SOLVED) &&
-                has_capability('mod/moodleoverflow:marksolved', $modulecontext))
-        ) {
-            throw new moodle_exception('rateownpost', 'moodleoverflow');
-        }
+        $authorcheck = ($post->userid == $userid) && ! (($rating == RATING_SOLVED || $rating == RATING_REMOVE_SOLVED) &&
+                                                        has_capability('mod/moodleoverflow:marksolved', $modulecontext));
+        moodleoverflow_throw_exception_with_check($authorcheck, 'rateownpost');
 
         // Check if we are removing a mark.
         if (in_array($rating / 10, $possibleratings)) {
@@ -121,15 +105,13 @@ public static function moodleoverflow_add_rating($moodleoverflow, $postid, $rati
         // Mark a post as solution or as helpful.
         if ($rating == RATING_SOLVED || $rating == RATING_HELPFUL) {
 
-            // Check if the current user is the startuser.
-            if ($rating == RATING_HELPFUL && $userid != $discussion->userid) {
-                throw new moodle_exception('notstartuser', 'moodleoverflow');
-            }
+            // Make sure that a helpful mark is made by the user who started the discussion.
+            $isnotstartuser = $rating == RATING_HELPFUL && $userid != $discussion->userid;
+            moodleoverflow_throw_exception_with_check($isnotstartuser, 'nostartuser');
 
-            // Check if the current user is a teacher.
-            if ($rating == RATING_SOLVED && !has_capability('mod/moodleoverflow:marksolved', $modulecontext)) {
-                throw new moodle_exception('notteacher', 'moodleoverflow');
-            }
+            // Make sure that a solution mark is made by a teacher (or someone with the right capability).
+            $isnotteacher = $rating == RATING_SOLVED && !has_capability('mod/moodleoverflow:marksolved', $modulecontext);
+            moodleoverflow_throw_exception_with_check($isnotteacher, 'notteacher');
 
             // Check if multiple marks are not enabled.
             if (!$multiplemarks) {
@@ -145,22 +127,18 @@ public static function moodleoverflow_add_rating($moodleoverflow, $postid, $rati
                     return self::moodleoverflow_update_rating_record($post->id, $rating, $userid, $otherrating->id, $modulecontext);
 
                 } else {
-                    $mid = $moodleoverflow->id;
-
-                    return self::moodleoverflow_add_rating_record($mid, $discussion->id, $post->id,
+                    return self::moodleoverflow_add_rating_record($moodleoverflow->id, $discussion->id, $post->id,
                                                                   $rating, $userid, $modulecontext);
                 }
-
             } else {
                 // If multiplemarks are allowed, only create a new rating.
-                $mid = $moodleoverflow->id;
-                return self::moodleoverflow_add_rating_record($mid, $discussion->id, $post->id, $rating, $userid, $modulecontext);
+                return self::moodleoverflow_add_rating_record($moodleoverflow->id, $discussion->id, $post->id,
+                                                              $rating, $userid, $modulecontext);
             }
         }
 
         // Update an rating record.
         if ($oldrating['normal']) {
-
             moodleoverflow_get_config_or_exception('moodleoverflow', 'allowratingchange',
                                                    'noratingchangeallowed', 'moodleoverflow');
 
@@ -174,35 +152,25 @@ public static function moodleoverflow_add_rating($moodleoverflow, $postid, $rati
         }
 
         // Create a new rating record.
-        $mid = $moodleoverflow->id;
-        $did = $post->discussion;
-
-        return self::moodleoverflow_add_rating_record($mid, $did, $postid, $rating, $userid, $modulecontext);
+        return self::moodleoverflow_add_rating_record($moodleoverflow->id, $post->discussion, $postid,
+                                                      $rating, $userid, $modulecontext);
     }
 
     /**
      * Get the reputation of a user.
      * Whether within a course or an instance is decided by the settings.
      *
      * @param int  $moodleoverflowid
-     * @param null $userid
+     * @param int $userid
      * @param bool $forcesinglerating If true you only get the reputation for the given $moodleoverflowid,
      * even if coursewidereputation = true
      *
      * @return int
      */
-    public static function moodleoverflow_get_reputation($moodleoverflowid, $userid = null, $forcesinglerating = false) {
-        global $DB, $USER;
-
-        // Get the user id.
-        if (!isset($userid)) {
-            $userid = $USER->id;
-        }
-
+    public static function moodleoverflow_get_reputation($moodleoverflowid, $userid, $forcesinglerating = false) {
         // Check the moodleoverflow instance.
-        if (!$moodleoverflow = $DB->get_record('moodleoverflow', ['id' => $moodleoverflowid])) {
-            throw new moodle_exception('invalidmoodleoverflowid', 'moodleoverflow');
-        }
+        $moodleoverflow = moodleoverflow_get_record_or_exception('moodleoverflow', ['id' => $moodleoverflowid],
+                                                                 'invalidmoodleoverflowid');
 
         // Check whether the reputation can be summed over the whole course.
         if ($moodleoverflow->coursewidereputation && !$forcesinglerating) {
@@ -417,39 +385,38 @@ public static function moodleoverflow_get_reputation_instance($moodleoverflowid,
         // Initiate a variable.
         $reputation = 0;
 
-        if ($moodleoverflow->anonymous != anonymous::EVERYTHING_ANONYMOUS) {
-            // Get all posts of this user in this module.
-            // Do not count votes for own posts.
-            $sql = "SELECT r.id, r.postid as post, r.rating
-                  FROM {moodleoverflow_posts} p
-                  JOIN {moodleoverflow_ratings} r ON p.id = r.postid
-                 WHERE p.userid = ? AND NOT r.userid = ? AND r.moodleoverflowid = ? ";
+        // Get all posts of this user in this module.
+        // Do not count votes for own posts.
+        $sql = "SELECT r.id, r.postid as post, r.rating
+              FROM {moodleoverflow_posts} p
+              JOIN {moodleoverflow_ratings} r ON p.id = r.postid
+              JOIN {moodleoverflow} m ON r.moodleoverflowid = m.id
+             WHERE p.userid = ? AND NOT r.userid = ? AND r.moodleoverflowid = ? AND m.anonymous <> ?";
 
-            if ($moodleoverflow->anonymous == anonymous::QUESTION_ANONYMOUS) {
-                $sql .= " AND p.parent <> 0 ";
-            }
+        if ($moodleoverflow->anonymous == anonymous::QUESTION_ANONYMOUS) {
+            $sql .= " AND p.parent <> 0 ";
+        }
 
-            $sql .= "ORDER BY r.postid ASC";
-
-            $params = [$userid, $userid, $moodleoverflowid];
-            $records = $DB->get_records_sql($sql, $params);
-
-            // Iterate through all ratings.
-            foreach ($records as $record) {
-                switch ($record->rating) {
-                    case RATING_DOWNVOTE:
-                        $reputation += get_config('moodleoverflow', 'votescaledownvote');
-                        break;
-                    case RATING_UPVOTE:
-                        $reputation += get_config('moodleoverflow', 'votescaleupvote');
-                        break;
-                    case RATING_HELPFUL:
-                        $reputation += get_config('moodleoverflow', 'votescalehelpful');
-                        break;
-                    case RATING_SOLVED:
-                        $reputation += get_config('moodleoverflow', 'votescalesolved');
-                        break;
-                }
+        $sql .= "ORDER BY r.postid ASC";
+
+        $params = [$userid, $userid, $moodleoverflowid, anonymous::EVERYTHING_ANONYMOUS];
+        $records = $DB->get_records_sql($sql, $params);
+
+        // Iterate through all ratings.
+        foreach ($records as $record) {
+            switch ($record->rating) {
+                case RATING_DOWNVOTE:
+                    $reputation += get_config('moodleoverflow', 'votescaledownvote');
+                    break;
+                case RATING_UPVOTE:
+                    $reputation += get_config('moodleoverflow', 'votescaleupvote');
+                    break;
+                case RATING_HELPFUL:
+                    $reputation += get_config('moodleoverflow', 'votescalehelpful');
+                    break;
+                case RATING_SOLVED:
+                    $reputation += get_config('moodleoverflow', 'votescalesolved');
+                    break;
             }
         }
 
@@ -527,33 +494,29 @@ private static function moodleoverflow_check_old_rating($postid, $userid, $oldra
         // Initiate the array.
         $rating = [];
 
-        // Get the normal rating.
         $sql = "SELECT *
-                FROM {moodleoverflow_ratings}
-                WHERE userid = ? AND postid = ? AND (rating = 1 OR rating = 2)";
-        $rating['normal'] = $DB->get_record_sql($sql, [ $userid, $postid ]);
+                FROM {moodleoverflow_ratings}";
+        // Get the normal rating.
+        $condition = "WHERE userid = ? AND postid = ? AND (rating = 1 OR rating = 2)";
+        $rating['normal'] = $DB->get_record_sql($sql . $condition, [ $userid, $postid ]);
 
         // Return the rating if it is requested.
         if ($oldrating == RATING_DOWNVOTE || $oldrating == RATING_UPVOTE) {
             return $rating['normal'];
         }
 
         // Get the solved rating.
-        $sql = "SELECT *
-                FROM {moodleoverflow_ratings}
-                WHERE postid = ? AND rating = 3";
-        $rating['solved'] = $DB->get_record_sql($sql, [ $postid ]);
+        $condition = "WHERE postid = ? AND rating = 3";
+        $rating['solved'] = $DB->get_record_sql($sql . $condition, [ $postid ]);
 
         // Return the rating if it is requested.
         if ($oldrating == RATING_SOLVED) {
             return $rating['solved'];
         }
 
         // Get the helpful rating.
-        $sql = "SELECT *
-                FROM {moodleoverflow_ratings}
-                WHERE postid = ? AND rating = 4";
-        $rating['helpful'] = $DB->get_record_sql($sql, [ $postid ]);
+        $condition = "WHERE postid = ? AND rating = 4";
+        $rating['helpful'] = $DB->get_record_sql($sql . $condition, [ $postid ]);
 
         // Return the rating if it is requested.
         if ($oldrating == RATING_HELPFUL) {
@@ -585,7 +548,6 @@ private static function moodleoverflow_can_be_changed($postid, $rating, $userid)
 
     /**
      * Removes a rating record.
-     *
      * @param int             $postid
      * @param int             $rating
      * @param int             $userid
@@ -605,11 +567,7 @@ private static function moodleoverflow_remove_rating($postid, $rating, $userid,
         $oldrecord = self::moodleoverflow_check_old_rating($postid, $userid, $rating);
 
         // Trigger an event.
-        $params = [
-            'objectid' => $oldrecord->id,
-            'context' => $modulecontext,
-        ];
-        $event = \mod_moodleoverflow\event\rating_deleted::create($params);
+        $event = \mod_moodleoverflow\event\rating_deleted::create(['objectid' => $oldrecord->id, 'context' => $modulecontext]);
         $event->add_record_snapshot('moodleoverflow_ratings', $oldrecord);
         $event->trigger();
 

discussion.php

@@ -21,6 +21,9 @@
  * @copyright 2017 Kennet Winter <[email protected]>
  * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
+defined('MOODLE_INTERNAL') || die();
+
+global $CFG, $PAGE, $USER, $SESSION, $OUTPUT;
 
 // Include config and locallib.
 require_once('../../config.php');
@@ -83,7 +86,7 @@
 
     if (in_array($ratingid, [RATING_SOLVED, RATING_REMOVE_SOLVED, RATING_HELPFUL, RATING_REMOVE_HELPFUL])) {
         // Rate the post.
-        if (!\mod_moodleoverflow\ratings::moodleoverflow_add_rating($moodleoverflow, $ratedpost, $ratingid, $cm)) {
+        if (!\mod_moodleoverflow\ratings::moodleoverflow_add_rating($moodleoverflow, $ratedpost, $ratingid, $cm, $USER->id)) {
             throw new moodle_exception('ratingfailed', 'moodleoverflow');
         }
 

externallib.php

@@ -114,7 +114,7 @@ public static function record_vote($postid, $ratingid) {
 
         // Rate the post.
         if (!\mod_moodleoverflow\ratings::moodleoverflow_add_rating($moodleoverflow,
-            $params['postid'], $params['ratingid'], $cm)) {
+            $params['postid'], $params['ratingid'], $cm, $USER->id)) {
             throw new moodle_exception('ratingfailed', 'moodleoverflow');
         }
 

locallib.php

@@ -2188,14 +2188,14 @@ function moodleoverflow_quick_array_sort(&$array, $low, $high, $key, $order) {
  * @param array $options                Conditions for the record
  * @param string $exceptionstring       Name of the moodleoverflow exception that should be thrown in case there is no record.
  * @param string $fields                Optional fields that are retrieved from the found record.
- * @param bool $coreexception           Optional param if exception from the core exceptions.
+ * @param bool $coreexception           Optional param if exception is from the core exceptions.
  * @return mixed $record                The found record
  */
 function moodleoverflow_get_record_or_exception($table, $options, $exceptionstring, $fields = '*', $coreexception = false) {
     global $DB;
     if (!$record = $DB->get_record($table, $options, $fields)) {
         if ($coreexception) {
-            throw new moodle_exception('invalidcourseid');
+            throw new moodle_exception($exceptionstring);
         } else {
             throw new moodle_exception($exceptionstring, 'moodleoverflow');
         }
@@ -2217,3 +2217,41 @@ function moodleoverflow_get_config_or_exception($plugin, $configname, $errorcode
     }
     return $config;
 }
+
+/**
+ * Function that throws an exception if a given check is true.
+ * @param bool $check               The result of a boolean check.
+ * @param string $errorcode         Error code/name of the exception
+ * @param string $coreexception     Optional param if exception is from the core exceptions and not moodleoverflow.
+ * @return void
+ */
+function moodleoverflow_throw_exception_with_check($check, $errorcode, $coreexception = false) {
+    if ($check) {
+        if ($coreexception) {
+            throw new moodle_exception($errorcode);
+        } else {
+            throw new moodle_exception($errorcode, 'moodleoverflow');
+        }
+    }
+}
+
+/**
+ * Function that catches unenrolled users and redirects them to the enrolment page.
+ * @param context $coursecontext     The context of the course.
+ * @param int $courseid             Id of the course that the user needs to enrol.
+ * @param string $returnurl         The url to return to after the user has been enrolled.
+ * @return void
+ */
+function moodleoverflow_catch_unenrolled_user($coursecontext, $courseid, $returnurl) {
+    global $SESSION;
+    if (!isguestuser() && !is_enrolled($coursecontext)) {
+        if (enrol_selfenrol_available($courseid)) {
+            $SESSION->wantsurl = qualified_me();
+            $SESSION->enrolcancel = get_local_referer(false);
+            redirect(new \moodle_url('/enrol/index.php', [
+                'id' => $courseid,
+                'returnurl' => $returnurl,
+            ]), get_string('youneedtoenrol'));
+        }
+    }
+}