WR-437576-disable-and-delete-users

Peer review feedback changes

Author: waleedhassan5

README.md

@@ -30,3 +30,7 @@ After installation, configure the plugin settings:
 ## Testing
 
 The plugin includes PHPUnit tests to ensure functionality. To run the tests, navigate to the Moodle root directory and execute:
+
+```bash
+vendor/bin/phpunit tests/task/cleanup_test.php
+```

classes/util.php

@@ -36,43 +36,19 @@
  */
 class util {
     /**
-     * Get the list of roles that should be excluded from the cleanup process.
-     *
-     * Users with these roles will not be subject to automatic disabling or deletion,
-     * even if they also have a student role.
-     *
-     * @return array Array of role shortnames that are excluded from processing
-     */
-    public static function get_excluded_roles(): array {
-        return ['coursecreator', 'editingteacher', 'teacher', 'manager', 'admin'];
-    }
-
-    /**
-     * Check if a user has any roles that exclude them from cleanup.
+     * Check if a user has the protected account capability.
      *
      * @param int $userid The ID of the user to check
-     * @return bool True if the user has any excluded roles, false otherwise
+     * @return bool True if the user has the protected account capability, false otherwise
      */
-    public static function has_excluded_roles(int $userid): bool {
-        global $DB;
-
-        $excludedroles = self::get_excluded_roles();
-        $roles = get_user_roles(\context_system::instance(), $userid);
-
-        foreach ($roles as $role) {
-            if (in_array($role->shortname, $excludedroles)) {
-                return true;
-            }
-        }
-
-        return false;
+    public static function has_protected_account_capability(int $userid): bool {
+        return user_has_capability('tool/disable_delete_students:protected', \context_system::instance(), $userid);
     }
 
     /**
      * Process all student accounts for potential disabling or deletion.
      *
      * This method implements the main business logic for account management:
-     * - Identifies active student accounts
      * - Checks each account against configured timeframes
      * - Disables accounts that meet the disability criteria
      * - Deletes accounts that meet the deletion criteria
@@ -96,8 +72,8 @@ public static function process_student_accounts() {
         $students = $DB->get_records_sql($sql);
 
         foreach ($students as $student) {
-            // Skip if user has excluded roles.
-            if (self::has_excluded_roles($student->id)) {
+            // Skip if user has protected account capability.
+            if (self::has_protected_account_capability($student->id)) {
                 continue;
             }
 

db/access.php

@@ -0,0 +1,12 @@
+$capabilities = [
+    'tool/disable_delete_students:protected' => [
+        'captype' => 'read',
+        'contextlevel' => CONTEXT_SYSTEM,
+        'archetypes' => [
+            'manager' => CAP_ALLOW,
+            'editingteacher' => CAP_ALLOW,
+            'teacher' => CAP_ALLOW,
+            'admin' => CAP_ALLOW,
+        ],
+    ],
+];

lang/en/tool_disable_delete_students.php

@@ -27,11 +27,13 @@
  * @author     Waleed ul hassan <[email protected]>
  */
 
-$string['pluginname'] = 'Student Account Cleanup';
+$string['pluginname'] = 'Disable Delete Accounts';
 $string['taskname'] = 'Clean up student accounts';
 $string['disable_after_course_end'] = 'Days after course end to disable';
 $string['disable_after_course_end_desc'] = 'Number of days after course end to disable student accounts';
 $string['disable_after_creation'] = 'Days after creation to disable';
 $string['disable_after_creation_desc'] = 'Number of days after account creation to disable student accounts';
 $string['delete_after_months'] = 'Months after course end to delete';
 $string['delete_after_months_desc'] = 'Number of months after course end to delete student accounts';
+$string['protectedaccounts'] = 'Protected Accounts';
+$string['protectedaccounts_desc'] = 'Accounts with the protected capability will not be disabled or deleted.';

settings.php

@@ -33,26 +33,10 @@
     $settings->add(new admin_setting_configtext(
         'tool_disable_delete_students/disable_after_course_end',
         get_string('disable_after_course_end', 'tool_disable_delete_students'),
-        get_string('disable_after_course_end_desc', 'tool_disable_delete_students'),
-        21,
-        PARAM_INT
+        get_string('configdisable_after_course_end', 'tool_disable_delete_students'),
+        30 // Default value
     ));
 
-    // Days after creation to disable account.
-    $settings->add(new admin_setting_configtext(
-        'tool_disable_delete_students/disable_after_creation',
-        get_string('disable_after_creation', 'tool_disable_delete_students'),
-        get_string('disable_after_creation_desc', 'tool_disable_delete_students'),
-        45,
-        PARAM_INT
-    ));
-
-    // Months after course end to delete account.
-    $settings->add(new admin_setting_configtext(
-        'tool_disable_delete_students/delete_after_months',
-        get_string('delete_after_months', 'tool_disable_delete_students'),
-        get_string('delete_after_months_desc', 'tool_disable_delete_students'),
-        6,
-        PARAM_INT
-    ));
+    // Add a section to explain the protected account capability
+    $settings->add(new admin_setting_heading('protected_accounts', get_string('protectedaccounts', 'tool_disable_delete_students'), get_string('protectedaccounts_desc', 'tool_disable_delete_students')));
 }

tests/generator/lib.php

@@ -14,6 +14,8 @@
 // You should have received a copy of the GNU General Public License
 // along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
 
+namespace tool_disable_delete_students;
+
 /**
  * Generator for the tool_disable_delete_students plugin.
  *
@@ -24,12 +26,6 @@
  * @copyright  2024 onwards Catalyst IT {@link http://www.catalyst-eu.net/}
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  * @author     Waleed ul hassan <[email protected]>
- */
-
-/**
- * Tool disable_delete_students generator
- *
- * @package    tool_disable_delete_students
  * @category   test
  */
 class tool_disable_delete_students_generator extends testing_module_generator {
@@ -44,25 +40,6 @@ public function create_test_course($record = null) {
         if (!isset($record['startdate'])) {
             $record['startdate'] = time();
         }
-        return $this->getDataGenerator()->create_course($record);
-    }
-
-    /**
-     * Create a test user with a specific role.
-     *
-     * @param string $role The role to assign to the user.
-     * @param array|null $record Optional settings for the user.
-     * @return stdClass User record.
-     * @throws dml_exception|coding_exception If there is a database error while assigning the role.
-     */
-    public function create_test_user_with_role($role, $record = null) {
-        global $DB;
-        $user = $this->getDataGenerator()->create_user($record);
-        $systemcontext = \context_system::instance();
-        $rolerecord = $DB->get_record('role', ['shortname' => $role]);
-        if ($rolerecord) {
-            role_assign($rolerecord->id, $user->id, $systemcontext->id);
-        }
-        return $user;
+        // Additional logic for creating the course...
     }
 }