To start, you should probably code a bit more defensively – i.e. add some checks to only execute the logic if there's actually a location
POST param for the current request, if a group by that handle actually exist, etc.
You should also add a check to avoid running this logic for console requests. As is, the current code would throw an exception if users are (re)saved via Craft's CLI, since console requests don't actually have a getBodyParam()
method.
And finally, it's probably good to know that the way Craft::$app->getUsers()->assignUserToGroups()
works, is that it will actually remove any current group affiliations, if those groups are not also passed in. I.e. if a user is already in groups A and B, and you do Craft::$app->getUsers()->assignUserToGroups($user->id, [$group->id])
to add the user to a group C, that user will be removed from groups A and B.
I'm not sure if that's how you want it to work, but assuming it's not, you'll need to make sure to include the IDs to all other groups that the member is already a member of, as well.
(The above is why you're seeing users losing their group affiliations when saved; you're probably passing an empty array to the assignUserToGroups()
most of the time, which - by design - will remove all group memberships for that user.)
Putting all of those together, here's how it could look:
Event::on(
User::class,
Element::EVENT_AFTER_SAVE,
function (ModelEvent $event) {
// Avoid running this code for console requests
if (Craft::$app->getRequest()->getIsConsoleRequest()) {
return;
}
// Get the "location" parameter
$location = Craft::$app->getRequest()->getBodyParam('location');
// Don't do anything if there isn't actually "location" POST parameter
if (!$location) {
return;
}
// Get the group to add the user to
$group = Craft::$app->getUserGroups()->getGroupByHandle($location);
// Don't do anything if no group was found by that "location" handle
if (!$group) {
return;
}
// Get the user
$user = $event->sender;
// Get the IDs for all the groups the user is currently a member of
$currentGroupIds = \array_map(function ($group) {
return (int)$group->id;
}, $user->getGroups());
// Don't do anything if the user is already in the given "location" group
if (in_array((int)$group->id, $currentGroupIds)) {
return;
}
// Add the user to the new group
// If you want the user to *only* be a member of the given "location" group, skip this part and just do `Craft::$app->getUsers()->assignUserToGroups($user->id, [$group->id]);` like you had before
$allGroupIds = array_merge($currentGroupIds, [$group->id]);
Craft::$app->getUsers()->assignUserToGroups($user->id, $allGroupIds);
}
);