Skip to content
This repository has been archived by the owner on May 7, 2020. It is now read-only.

Commit

Permalink
Autogroup Bug
Browse files Browse the repository at this point in the history
  • Loading branch information
sergei-porfenovich committed Aug 2, 2019
1 parent e674152 commit 0b29541
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 10 deletions.
33 changes: 26 additions & 7 deletions classes/domain/autogroup_set.php
Original file line number Diff line number Diff line change
Expand Up @@ -295,18 +295,27 @@ public function verify_user_group_membership(\stdclass $user, \moodle_database $

//an array of groupids which will be populated as we ensure membership
$validgroups = array();
$new_group = false;

foreach ($eligiblegroups as $eligiblegroup){
if($group = $this->get_or_create_group_by_idnumber($eligiblegroup, $db)) {
list($group, $group_created) = $this->get_or_create_group_by_idnumber($eligiblegroup, $db);
if($group) {
$validgroups[] = $group->id;
$group->ensure_user_is_member($user->id);
if ($group->courseid == $this->courseid){
if (!$new_group or $group_created){
$new_group = $group->id;
}
}
}
}

//now run through other groups and ensure user is not a member
foreach($this->groups as $key => $group){
if(!in_array($group->id,$validgroups)){
$group->ensure_user_is_not_member($user->id);
if($group->ensure_user_is_not_member($user->id) and $new_group){
$this->check_forums($user->id, $group->id, $new_group, $db);
}
}
}

Expand Down Expand Up @@ -358,7 +367,7 @@ private function get_autogroups(\moodle_database $db)
/**
* @param string $groupname
* @param \moodle_database $db
* @return bool|domain/group
* @return [bool|domain/group, bool]
*/
private function get_or_create_group_by_idnumber($group, \moodle_database $db)
{
Expand All @@ -372,7 +381,6 @@ private function get_or_create_group_by_idnumber($group, \moodle_database $db)
}

$idnumber = $this->generate_group_idnumber($groupidnumber);
$result = null;

//firstly run through existing groups and check for matches
foreach($this->groups as $group){
Expand All @@ -383,7 +391,7 @@ private function get_or_create_group_by_idnumber($group, \moodle_database $db)
$group->update();
}

return $group;
return [$group, false];
}
}

Expand All @@ -404,10 +412,10 @@ private function get_or_create_group_by_idnumber($group, \moodle_database $db)
$newgroup->create();
$this->groups[$newgroup->id] = $newgroup;
} catch (exception\invalid_group_argument $e){
return false;
return [false, false];
}

return $this->groups[$newgroup->id];
return [$this->groups[$newgroup->id], true];
}

/**
Expand Down Expand Up @@ -602,6 +610,17 @@ private function validate_object($autogroupset)
&& $autogroupset->courseid > 0;
}

/**
* @param int $user_id
* @param int $old_group_id
* @param int $new_group_id
* @param \moodle_database $db
*/
private function check_forums($user_id, $old_group_id, $new_group_id, \moodle_database $db){
$conditions = ['course' => $this->courseid, 'userid' => $user_id, 'groupid' => $old_group_id];
$db->set_field('forum_discussions', 'groupid', $new_group_id, $conditions);
}

/**
* An array of DB level attributes for an autogroup set
* used for handling stdclass object conversion.
Expand Down
8 changes: 6 additions & 2 deletions classes/domain/group.php
Original file line number Diff line number Diff line change
Expand Up @@ -73,28 +73,32 @@ public function __construct ($group, \moodle_database $db)

/**
* @param int $userid
* @return bool
*/
public function ensure_user_is_member($userid){
foreach($this->members as $member){
if ($member == $userid) {
return;
return false;
}
}

//user was not found as a member so will now make member a user
\groups_add_member($this->as_object(), $userid, 'local_autogroup');
return;
return true;
}

/**
* @param int $userid
* @return bool
*/
public function ensure_user_is_not_member($userid){
foreach($this->members as $member){
if ($member == $userid) {
\groups_remove_member($this->as_object(), $userid);
return true;
}
}
return false;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion version.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@

defined('MOODLE_INTERNAL') || die();

$plugin->version = 2018070300;
$plugin->version = 2019080200;
$plugin->requires = 2013111800.00; // Requires this Moodle version (2.7).
$plugin->release = '2.4'; // Plugin release.
$plugin->component = 'local_autogroup'; // Full name of the plugin (used for diagnostics).
Expand Down

1 comment on commit 0b29541

@sergei-porfenovich
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moodle handles course groups and forum groups separately. If a group doesn’t have any members, Autogroup will delete the course group but it does not delete/update the groups associated with forum posts. As a result, these forum posts associated with the old groups will point to groups that no longer exist.

Now, when it changes user's group, it also changes groups of topics, where author is this user and topic's group is his old group - to user new group.

Please sign in to comment.