Fix `GroupsController#group_params` to allow more group attributes to be updated.

app/controllers/groups_controller.rb

@@ -133,9 +133,9 @@ class GroupsController < ApplicationController
 
   def update
     group = Group.find(params[:id])
-    guardian.ensure_can_edit!(group)
+    guardian.ensure_can_edit!(group) unless current_user.admin
 
-    if group.update_attributes(group_params)
+    if group.update(group_params(automatic: group.automatic))
       GroupActionLogger.new(current_user, group).log_change_group_settings
       DiscourseEvent.trigger(:group_updated, group)
 
@@ -426,22 +426,44 @@ class GroupsController < ApplicationController
 
   private
 
-  def group_params
+  def group_params(automatic: false)
-    permitted_params = [
+    permitted_params =
-      :flair_url,
+      if automatic
-      :flair_bg_color,
+        %i{
-      :flair_color,
+          visibility_level
-      :bio_raw,
+          mentionable_level
-      :full_name,
+          messageable_level
-      :public_admission,
+          default_notification_level
-      :public_exit,
+        }
-      :allow_membership_requests,
+      else
-      :membership_request_template,
+        default_params = %i{
-    ]
+          mentionable_level
+          messageable_level
+          visibility_level
+          automatic_membership_email_domains
+          automatic_membership_retroactive
+          title
+          primary_group
+          grant_trust_level
+          incoming_email
+          flair_url
+          flair_bg_color
+          flair_color
+          bio_raw
+          public_admission
+          public_exit
+          allow_membership_requests
+          full_name
+          default_notification_level
+          membership_request_template
+        }
 
-    if current_user.admin
+        if current_user.admin
-      permitted_params.push(:name)
+          default_params.push(:name)
-    end
+        end
+
+        default_params
+      end
 
     params.require(:group).permit(*permitted_params)
   end

spec/requests/groups_controller_spec.rb

@@ -393,12 +393,27 @@ describe GroupsController do
       end
 
       it "should be able update the group" do
-        group.update!(allow_membership_requests: false)
+        group.update!(
+          allow_membership_requests: false,
+          visibility_level: 2,
+          mentionable_level: 2,
+          messageable_level: 2,
+          default_notification_level: 0,
+          grant_trust_level: 0,
+        )
 
         expect do
           put "/groups/#{group.id}.json", params: {
             group: {
-              name: 'testing',
+              mentionable_level: 1,
+              messageable_level: 1,
+              visibility_level: 1,
+              automatic_membership_email_domains: 'test.org',
+              automatic_membership_retroactive: true,
+              title: 'haha',
+              primary_group: true,
+              grant_trust_level: 1,
+              incoming_email: 'test@mail.org',
               flair_bg_color: 'FFF',
               flair_color: 'BBB',
               flair_url: 'fa-adjust',
@@ -408,9 +423,11 @@ describe GroupsController do
               public_exit: true,
               allow_membership_requests: true,
               membership_request_template: 'testing',
+              default_notification_level: 1,
+              name: 'testing'
             }
           }
-        end.to change { GroupHistory.count }.by(9)
+        end.to change { GroupHistory.count }.by(19)
 
         expect(response.status).to eq(200)
 
@@ -425,8 +442,17 @@ describe GroupsController do
         expect(group.public_exit).to eq(true)
         expect(group.allow_membership_requests).to eq(true)
         expect(group.membership_request_template).to eq('testing')
-        expect(GroupHistory.last.subject).to eq('membership_request_template')
         expect(group.name).to eq('test')
+        expect(group.visibility_level).to eq(1)
+        expect(group.mentionable_level).to eq(1)
+        expect(group.messageable_level).to eq(1)
+        expect(group.default_notification_level).to eq(1)
+        expect(group.automatic_membership_email_domains).to eq('test.org')
+        expect(group.automatic_membership_retroactive).to eq(true)
+        expect(group.title).to eq('haha')
+        expect(group.primary_group).to eq(true)
+        expect(group.incoming_email).to eq("test@mail.org")
+        expect(group.grant_trust_level).to eq(1)
       end
     end
 
@@ -451,6 +477,38 @@ describe GroupsController do
         expect(group.name).to eq('testing')
       end
 
+      it "should be able to update an automatic group" do
+        group = Group.find(Group::AUTO_GROUPS[:admins])
+
+        group.update!(
+          visibility_level: 2,
+          mentionable_level: 2,
+          messageable_level: 2,
+          default_notification_level: 2
+        )
+
+        put "/groups/#{group.id}.json", params: {
+          group: {
+            flair_color: 'BBB',
+            name: 'testing',
+            visibility_level: 1,
+            mentionable_level: 1,
+            messageable_level: 1,
+            default_notification_level: 1
+          }
+        }
+
+        expect(response.status).to eq(200)
+
+        group.reload
+        expect(group.flair_color).to eq(nil)
+        expect(group.name).to eq('admins')
+        expect(group.visibility_level).to eq(1)
+        expect(group.mentionable_level).to eq(1)
+        expect(group.messageable_level).to eq(1)
+        expect(group.default_notification_level).to eq(1)
+      end
+
       it 'triggers a extensibility event' do
         event = DiscourseEvent.track_events {
           put "/groups/#{group.id}.json", params: { group: { flair_color: 'BBB' } }