Fix a few issues

Add back "redundant" return value descriptions in docblocks
Improve readability slightly

classes/condition.php

@@ -67,6 +67,7 @@ class condition extends abstract_condition {
      * @param bool $grabthelot Performance hint: if true, caches information required for all course-modules, to make the front page
      *                         and similar pages work more quickly (works only for current user)
      * @param int $userid User ID to check availability for
+     * @return bool True if available
      * @throws dml_exception
      */
     public function is_available($not, info $info, $grabthelot, $userid): bool {
@@ -84,6 +85,7 @@ public function is_available($not, info $info, $grabthelot, $userid): bool {
      * @param bool $full Set true if this is the 'full information' view
      * @param bool $not Set true if we are inverting the condition
      * @param info $info Item we're checking
+     * @return string Information string (for admin) about all restrictions on this item
      * @throws moodle_exception
      */
     public function get_description($full, $not, info $info): string {

classes/event/user_2fa_loggedin.php

@@ -66,7 +66,7 @@ public static function get_name(): string {
     }
 
     /**
-     * Returns a description of what happened.
+     * Returns non-localised event description with id's for admin use only.
      *
      * @return string
      */

classes/exception_current_user_selector.php

@@ -56,6 +56,10 @@ public function __construct($name, $options) {
      * {@inheritDoc}
      *
      * @param string $search the search string.
+     * @return array An array of arrays of users. The array keys of the outer array should be the string names of optgroups.
+     *               The keys of the inner arrays should be userids, and the values should be user objects containing at least
+     *               the list of fields returned by the method required_fields_sql(). If a user object has a ->disabled property
+     *               that is true, then that option will be displayed greyed out, and will not be returned by get_selected_users.
      * @throws coding_exception
      * @throws dml_exception
      */

classes/exception_potential_user_selector.php

@@ -57,6 +57,10 @@ public function __construct($name, $options) {
      * {@inheritDoc}
      *
      * @param string $search the search string.
+     * @return array An array of arrays of users. The array keys of the outer array should be the string names of optgroups.
+     *               The keys of the inner arrays should be userids, and the values should be user objects containing at least
+     *               the list of fields returned by the method required_fields_sql(). If a user object has a ->disabled property
+     *               that is true, then that option will be displayed greyed out, and will not be returned by get_selected_users.
      * @throws coding_exception
      * @throws dml_exception
      */

classes/frontend.php

@@ -47,19 +47,20 @@ class frontend extends abstract_frontend {
     /**
      * Returns a list of language strings to pass to the javascript.
      *
-     * @return string[]
+     * @return string[] Array of required string identifiers
      */
     protected function get_javascript_strings(): array {
         return ['fulltitle'];
     }
 
     /**
      * Check if the condition can be added.
+     *
      * Can only be added if the user has the appropriate capability.
      *
-     * @param stdClass $course
-     * @param cm_info|null $cm (optional)
-     * @param section_info|null $section (optional)
+     * @param stdClass $course Course object
+     * @param cm_info|null $cm Course-module currently being edited (null if none)
+     * @param section_info|null $section Section currently being edited (null if none)
      * @return bool
      * @throws coding_exception
      */

classes/privacy/provider.php

@@ -55,6 +55,7 @@ class provider implements core_userlist_provider, metadata_provider, request_plu
      * {@inheritDoc}
      *
      * @param metadata_collection $collection The initialised collection to add items to.
+     * @return metadata_collection A listing of user data stored through this system.
      */
     public static function get_metadata(metadata_collection $collection): metadata_collection {
         $collection->add_database_table(
@@ -73,6 +74,7 @@ public static function get_metadata(metadata_collection $collection): metadata_c
      * {@inheritDoc}
      *
      * @param int $userid The user to search.
+     * @return contextlist The contextlist containing the list of contexts used in this plugin.
      */
     public static function get_contexts_for_userid(int $userid): contextlist {
         $contextlist = new contextlist();
@@ -97,20 +99,23 @@ public static function get_contexts_for_userid(int $userid): contextlist {
      */
     public static function export_user_data(approved_contextlist $contextlist): void {
         global $DB;
-        $user = $contextlist->get_user();
-        $userid = $user->id;
-        [$insql, $params] = $DB->get_in_or_equal($contextlist->get_contextids(), SQL_PARAMS_NAMED);
+        $contextids = $contextlist->get_contextids();
+        if (empty($contextids)) {
+            return;
+        }
+        $pluginname = get_string('pluginname', 'availability_shibboleth2fa');
+        [$insql, $params] = $DB->get_in_or_equal($contextids, SQL_PARAMS_NAMED);
         $sql = "SELECT ctx.id AS contextid, e.courseid, e.userid, e.skipauth
                   FROM {context} ctx
                   JOIN {course} crs ON crs.id = ctx.instanceid AND ctx.contextlevel = :contextlevel
                   JOIN {availability_shibboleth2fa_e} e ON e.courseid = crs.id
                  WHERE e.userid = :userid AND ctx.id $insql";
-        $params['userid'] = $userid;
+        $params['userid'] = $contextlist->get_user()->id;
         $params['contextlevel'] = CONTEXT_COURSE;
         $records = $DB->get_recordset_sql($sql, $params);
         foreach ($records as $record) {
             $context = context::instance_by_id($record->contextid);
-            writer::with_context($context)->export_data([get_string('pluginname', 'availability_shibboleth2fa')], $record);
+            writer::with_context($context)->export_data([$pluginname], $record);
         }
         $records->close();
     }
@@ -136,9 +141,6 @@ public static function delete_data_for_all_users_in_context(context $context): v
      */
     public static function delete_data_for_user(approved_contextlist $contextlist): void {
         global $DB;
-        if ($contextlist->count() === 0) {
-            return;
-        }
         $userid = $contextlist->get_user()->id;
         foreach ($contextlist->get_contexts() as $context) {
             if (is_a($context, context_course::class)) {

index.php

@@ -36,7 +36,11 @@
 $cmid = optional_param('cmid', null, PARAM_INT);
 $sectionid = optional_param('sectionid', null, PARAM_INT);
 
-[$course, $cm] = $cmid ? get_course_and_cm_from_cmid($cmid) : [get_course($courseid), null];
+if ($cmid) {
+    [$course, $cm] = get_course_and_cm_from_cmid($cmid);
+} else {
+    [$course, $cm] = [get_course($courseid), null];
+}
 
 $url = new moodle_url('/availability/condition/shibboleth2fa/index.php', ['id' => $course->id]);
 if ($cmid) {