2

I'm trying to add my users to a group based on the option they select in a dropdown field. I feel like I'm close but so far - at the moment, it's not adding the new user to a group and it's clearing other user's groups on updating their profile.

Can anyone point out where I'm going wrong?

Event::on(User::class, Element::EVENT_AFTER_SAVE, function (ModelEvent $event) {
    $user = $event->sender;
    $location = Craft::$app->getRequest()->getBodyParam('location');
    $group = Craft::$app->getUserGroups()->getGroupByHandle($location);
    Craft::$app->getUsers()->assignUserToGroups($user->id, [$group->id]);
});

I'm new to all of this so trying to learn! There might be other checks I should be doing that I'm not sure of...

1 Answer 1

3

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);
    
    }
);
1
  • 1
    This is amazing, exactly what I was looking for you perfectly answered what I'm still missing when thinking about these things in being defensive. The only minor issue, for anyone who stumbles on this, is that it won't work on new registration if you have a Default User Group selected in the User Settings. That wasn't essential for me, so I just removed it but I'm sure it would be easy enough to add to this. Thanks again!
    – brynli64
    Feb 23 at 0:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.