Skip to content
This repository has been archived by the owner on Apr 4, 2024. It is now read-only.

FEATURE: Add script to modify user group memberships through badges #206

Merged
merged 1 commit into from Aug 29, 2023

Conversation

s3lase
Copy link
Contributor

@s3lase s3lase commented Aug 25, 2023

This change adds a new script, user_group_membership_through_badge which provides the option to automate user group memberships through badges.

Given a badge name and a group, it adds users with the specified badged to the the given group either on login or on a recurring schedule.

It also has an optional flag to synchronize these two in "strict mode". It does this by ensuring only users with the specified badge belong to the given group.

Screenshot 2023-08-25 at 6 21 53 PM Screenshot 2023-08-25 at 6 22 40 PM

This change adds a new script, `user_group_membership_through_badge`
which provides the option to automate user group memberships through
badges.

Given a badge name and a group, this script adds users with the
specified badged to the the given group on login or on a recurring
schedule.

It also has an optional flag which when enabled ensures only users with the
specified badge are members of the given group

DiscourseAutomation::Scriptable::USER_GROUP_MEMBERSHIP_THROUGH_BADGE =
"user_group_membership_through_badge"
DiscourseAutomation::Scriptable::USER_GROUP_MEMBERSHIP_THROUGH_BADGE_BULK_MODIFY_START_COUNT = 1000
Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤷🏾 Arbitrary limit alert. I wonder if this is even needed, the add_user_to_group_through_custom_field script doesn't do this at the moment.

Better safe than sorry, we probably don't want to be doing 1000s of Group#add calls anyways

Comment on lines +56 to +65
if user_ids_to_add.count < bulk_modify_start_count
User
.where(id: user_ids_to_add)
.each do |user|
group.add(user)
GroupActionLogger.new(Discourse.system_user, group).log_add_user_to_group(user)
end
else
group.bulk_add(user_ids_to_add)
end
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we always do bulk_add ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How essential are the user_added_to_group and user_removed_from_group events? The bulk methods understandably don't trigger these.

I considered going all in with the bulk methods but decided against it for two main reasons:

  1. We lose the events and ability to trigger other automations off of the badge-group sync
  2. Compatibility with exisiting add_user_to_group_through_custom_field script. It does not perform bulk adds

Comment on lines +85 to +94
if user_ids_to_remove.count < bulk_modify_start_count
User
.where(id: user_ids_to_remove)
.each do |user|
group.remove(user)
GroupActionLogger.new(Discourse.system_user, group).log_remove_user_from_group(user)
end
else
group.bulk_remove(user_ids_to_remove)
end
Copy link
Contributor

Choose a reason for hiding this comment

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

same, maybe only bulk_remove ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as above

Copy link
Contributor

@jjaffeux jjaffeux left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@s3lase s3lase merged commit d8037f4 into main Aug 29, 2023
4 checks passed
@s3lase s3lase deleted the feature/user-group-membership-via-badge-script branch August 29, 2023 16:10
@s3lase
Copy link
Contributor Author

s3lase commented Aug 29, 2023

Thanks @jjaffeux

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
2 participants