Skip to content

Commit

Permalink
Fixes bug MDL-8233, "Delete course action, groups typos and groupings…
Browse files Browse the repository at this point in the history 
… not deleted"
  • Loading branch information
nfreear committed Jan 20, 2007
1 parent d8aa355 commit 2dee6ab
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 43 deletions.
9 changes: 5 additions & 4 deletions group/db/dbbasicgrouplib.php
View file
Expand Up @@ -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
*/
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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;
Expand Down
30 changes: 14 additions & 16 deletions group/lib/basicgrouplib.php
View file
Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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;
}


Expand All @@ -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.
Expand All @@ -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
Expand All @@ -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!
Expand All @@ -423,5 +422,4 @@ function groups_remove_all_members($groupid) {
return $success;
}


?>
50 changes: 30 additions & 20 deletions group/lib/groupinglib.php
View file
Expand Up @@ -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;
Expand All @@ -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.
Expand All @@ -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;
Expand Down Expand Up @@ -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) {
Expand All @@ -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;
}

Expand Down Expand Up @@ -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
Expand All @@ -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;
}

?>
12 changes: 9 additions & 3 deletions lib/moodlelib.php
View file
Expand Up @@ -2936,18 +2936,19 @@ 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');
}
} else {
$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) {
Expand All @@ -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
Expand Down

0 comments on commit 2dee6ab

Please sign in to comment.