Fixes bug MDL-8233, "Delete course action, groups typos and groupings not deleted"

diff --git a/group/db/dbbasicgrouplib.php b/group/db/dbbasicgrouplib.php
index 023774ddb6e40e3a04b790e2045f848abcb416c5..6be3ea8118269eabc83526bf81a39d8c5fa680b8 100644 (file)
--- a/group/db/dbbasicgrouplib.php
+++ b/group/db/dbbasicgrouplib.php
@@ -356,7 +356,7 @@ function groups_db_remove_member($groupid, $userid) {
 
 
 /** 
- * Deletes a specified group
+ * Delete a specified group, first removing members and links with courses and groupings. 
  * @param int $groupid The group to delete
  * @return boolean True if deletion was successful, false otherwise
  */
@@ -365,8 +365,7 @@ function groups_db_delete_group($groupid) {
         $success = false;
     } else {
         $success = true;
-        // Get a list of the users for the group and delete them all from the 
-        // group
+        // Get a list of users for the group and remove them all.
 
         $userids = groups_db_get_members($groupid);
         if ($userids != false) {
@@ -378,7 +377,8 @@ function groups_db_delete_group($groupid) {
                }
         }
 
-        // Remove any groupings that the group belongs to     
+        // Remove any links with groupings to which the group belongs.
+        //TODO: dbgroupinglib also seems to delete these links - duplication?
            $groupingids = groups_get_groupings_for_group($groupid); 
            if ($groupingids != false) {
                    foreach($groupingids as $groupingid) {
@@ -390,6 +390,7 @@ function groups_db_delete_group($groupid) {
                    }
            }
 
+        // Remove links with courses.
                $results = delete_records('groups_courses_groups', 'groupid', $groupid);
                if ($results == false) {
                        $success = false;

diff --git a/group/lib/basicgrouplib.php b/group/lib/basicgrouplib.php
index 6d09c3cd9898d634b5a8f816ed11095f6e861616..b36ee8f1fc1e80e7a37fca4e9095a25d95999e25 100644 (file)
--- a/group/lib/basicgrouplib.php
+++ b/group/lib/basicgrouplib.php
@@ -55,7 +55,8 @@ function groups_get_members($groupid, $membertype = false) {
 }
 
 /**
- * Return member records, for backup.
+ * Get the user ID and time added for each member of a group, for backup4.
+ * @return array An array of member records.
  */
 function groups_get_member_records($groupid) {
     if (!$groupid) {
@@ -81,6 +82,10 @@ function groups_get_groups_for_user($userid, $courseid) {
     return $groupids;
 }
 
+/**
+ * Get the groups to which a user belongs in any course on site.
+ * @return array | false An array of the group IDs, or false on error.
+ */
 function groups_get_all_groups_for_user($userid) {
     $groups = get_records('groups_members', 'userid', $userid);
     if (! $groups) {
@@ -358,7 +363,7 @@ function groups_restore_member($member) {
     } else {
         $useradded = groups_db_add_member($member->groupid, $member->userid, $member->timeadded);
     }
-    return true;
+    return $useradded;
 }
 
 
@@ -368,7 +373,7 @@ function groups_restore_member($member) {
 
 
 /**
- * Deletes a group best effort
+ * Delete a group best effort, first removing members and links with courses and groupings. 
  * @param int $groupid The group to delete
  * @return boolean True if deletion was successful, false otherwise
  * See comment above on web service autoupdating. 
@@ -379,20 +384,9 @@ function groups_delete_group($groupid) {
     return $groupdeleted;
 }
 
-/*function groups_delete_groups($groupids) {
-    if (! $groupids) {
-        return false;
-    }
-    $success = true;
-    foreach ($groupids as $id) {
-        $success = $success && groups_db_delete_group($id);
-    }
-    return $success;
-}*/
-
 
 /**
- * Deletes the specified user from the specified group
+ * Deletes the link between the specified user and group.
  * @param int $groupid The group to delete the user from
  * @param int $userid The user to delete
  * @return boolean True if deletion was successful, false otherwise
@@ -406,6 +400,11 @@ function groups_remove_member($groupid, $userid) {
     return $success;
 }
 
+/**
+ * Removes all users from the specified group.
+ * @param int $groupid The ID of the group.
+ * @return boolean True for success, false otherwise.
+ */
 function groups_remove_all_members($groupid) {
     if (! groups_group_exists($groupid)) {
         //Woops, delete group last!
@@ -423,5 +422,4 @@ function groups_remove_all_members($groupid) {
     return $success;
 }
 
-
 ?>
\ No newline at end of file

diff --git a/group/lib/groupinglib.php b/group/lib/groupinglib.php
index fdb0c75caeb0f8c1b32e746831d6b88455145f21..515d90d1624123ce69d6d5c48207e8673ceee3e6 100644 (file)
--- a/group/lib/groupinglib.php
+++ b/group/lib/groupinglib.php
@@ -31,12 +31,6 @@ function groups_get_groupings($courseid) {
 
 
 function groups_get_grouping_records($courseid) {
-    /*$groupingids = groups_db_get_groupings($courseid);
-    if (! $groupingids) {
-        return false;
-    }
-    $groupings = groups_groupingids_to_groupings($groupingids);
-*/
     global $CFG;
     if (! $courseid) {
         return false;
@@ -50,7 +44,7 @@ function groups_get_grouping_records($courseid) {
 }
 
 /**
- * Gets a list of the groups in a specified grouping
+ * Gets a list of the group IDs in a specified grouping
  * @param int $groupingid The id of the grouping
  * @return array | false. An array of the ids of the groups, or false if there
  * are none or an error occurred.
@@ -59,6 +53,11 @@ function groups_get_groups_in_grouping($groupingid) {
     return groups_db_get_groups_in_grouping($groupingid);
 }
 
+/**
+ * Gets complete group-data for each group in a grouping.
+ * @param int $groupingid The ID of the grouping.
+ * @return array | false An array of group records, or false on error.
+ */
 function groups_get_groups_in_grouping_records($groupingid) {
     if (! $groupingid) {
         return false;
@@ -127,10 +126,7 @@ function groups_get_groups_not_in_grouping($groupingid, $courseid) {
  */
 function groups_get_groups_not_in_any_grouping($courseid) {
     global $CFG;
-/*Was: $sql = "SELECT g.id FROM {$CFG->prefix}groups AS g
-        WHERE g.id NOT IN 
-        (SELECT groupid FROM {$CFG->prefix}groups_groupings_groups)";
-*/
+
     $join = '';
     $where= '';
     if ($courseid) {
@@ -146,14 +142,7 @@ function groups_get_groups_not_in_any_grouping($courseid) {
 
     $records = get_records_sql($sql);
     $groupids = groups_groups_to_groupids($records, $courseid);
-    /*$groupids = array();
-    if ($records) {
-        foreach ($records as $r) {
-            $groupids[] = $r->id;
-        }
-    } else {
-        return false;
-    }*/
+
     return $groupids;
 }
 
@@ -418,7 +407,7 @@ function groups_set_grouping_for_coursemodule($groupingid, $coursemoduleid) {
  *****************************/
 
 /** 
- * Removes a specified group from a specified grouping. Note that this does 
+ * Removes a specified group from a grouping. Note that this does 
  * not delete the group. 
  * @param int $groupid The id of the group.
  * @param int $groupingid The id of the grouping
@@ -439,4 +428,25 @@ function groups_delete_grouping($groupingid) {
     
 }
 
+/**
+ * Delete all groupings from a course. Groups MUST be deleted first.
+ * TODO: If groups or groupings are to be shared between courses, think again!
+ * @param $courseid The course ID.
+ * @return boolean True if all deletes were successful, false otherwise.
+ */
+function groups_delete_all_groupings($courseid) {
+    if (! $courseid) {
+        return false;
+    }
+    $groupingids = groups_get_groupings($courseid);
+    if (! $groupingids) {
+        return true;
+    }
+    $success = true;
+    foreach ($groupingids as $gg_id) {
+        $success = $success && groups_db_delete_grouping($gg_id);
+    }
+    return $success;
+}
+
 ?>
\ No newline at end of file

diff --git a/lib/moodlelib.php b/lib/moodlelib.php
index 17624b99cc47a166ee9de64e7b31cbc4bebfcff0..e62b1b26f22ceaf0a652937a1b1b4c4acd1194f9 100644 (file)
--- a/lib/moodlelib.php
+++ b/lib/moodlelib.php
@@ -2936,10 +2936,11 @@ function remove_course_contents($courseid, $showfeedback=true) {
         }
     }
 
-/// Delete any groups, removing members first. TODO: check.
+/// Delete any groups, removing members and grouping/course links first.
+    //TODO: If groups or groupings are to be shared between courses, think again!
     if ($groupids = groups_get_groups($course->id)) {
         foreach ($groupids as $groupid) {
-            if (groups_remove_all_group_members($groupid)) {
+            if (groups_remove_all_members($groupid)) {
                 if ($showfeedback) {
                     notify($strdeleted .' groups_members');
                 }
@@ -2947,7 +2948,7 @@ function remove_course_contents($courseid, $showfeedback=true) {
                 $result = false;
             }
             /// Delete any associated context for this group ??
-            delete_context(CONTEXT_GROUP, $group->id);
+            delete_context(CONTEXT_GROUP, $groupid);
             
             if (groups_delete_group($groupid)) {
                 if ($showfeedback) {
@@ -2958,6 +2959,11 @@ function remove_course_contents($courseid, $showfeedback=true) {
             }
         }
     }
+/// Delete any groupings.
+    $result = groups_delete_all_groupings($course->id);
+    if ($result && $showfeedback) {
+        notify($strdeleted .' groupings');
+    }
 
 /// Delete all related records in other tables that may have a courseid
 /// This array stores the tables that need to be cleared, as