Admin.groups.manage.membership.automatic not replaced

martin · December 9, 2024, 1:41am

I think the site setting Moin mentioned does allow this, here is the relevant guardian check:

discourse/discourse/blob/4ddf7e61f99d42e22ad5ae36832882c734623614/lib/guardian/group_guardian.rb#L18-L24


      
          def can_admin_group?(group)
            is_admin? ||
              (
                SiteSetting.moderators_manage_categories_and_groups && is_moderator? && can_see?(group) &&
                  group.id != Group::AUTO_GROUPS[:admins]
              )
          end

But in the client, we are sending mixed messages

github.com

discourse/discourse/blob/4ddf7e61f99d42e22ad5ae36832882c734623614/app/assets/javascripts/discourse/app/models/user.js#L1091-L1095


      
          canManageGroup(group) {
            return group.get("automatic")
              ? false
              : group.get("can_admin_group") || group.get("is_group_owner");
          }

github.com

discourse/discourse/blob/4ddf7e61f99d42e22ad5ae36832882c734623614/app/assets/javascripts/discourse/app/controllers/group.js#L117-L124


      
          @discourseComputed("model", "model.automatic")
          canManageGroup(model, automatic) {
            return (
              this.currentUser &&
              (this.currentUser.canManageGroup(model) ||
                (model.can_admin_group && automatic))
            );
          }

Here is a followup PR to change to check canManageGroup, and also update the user model version of canManageGroup to be more sensible (the only place using that is from this group controller):

github.com/discourse/discourse

FIX: Allow anyone who can manage groups to see Automatic tooltip

discourse:main ← discourse:issue/manage-groups-followup

opened 01:40AM - 09 Dec 24 UTC

martin-brennan

+55 -30

Followup 35ecd0335c70e4f008ae86ec5804dc60dbf41101 Since we have the moderator…s_manage_categories_and_groups setting, more than admins can manage groups, so we need to allow others to see this Automatic tooltip as well. Also fixes an inconsistency with canManageGroup between the User model and Group controller, the latter is correct, allowing management of automatic groups if can_admin_group permission is true

Topic		Replies	Views
Improved Group Members Management Feature rfc , spec	31	11050	January 7, 2016
Adding owners/members UX is inconsistent and misleading UX	26	2893	April 20, 2018
Default admin doesn’t receive ‘staff’ or ‘admin’ group memberships at first Bug	13	714	October 10, 2022
Improving the Groups page for 1.7 Feature feedback	71	10296	January 18, 2017
Admins can still read anyone's PM's by downloading the database Feature privacy	19	5695	March 26, 2015

Admin.groups.manage.membership.automatic not replaced

Related topics