diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 6d370477456..ada17605d0c 100644 --- a/app/controllers/groups_controller.rb +++ b/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 - permitted_params = [ - :flair_url, - :flair_bg_color, - :flair_color, - :bio_raw, - :full_name, - :public_admission, - :public_exit, - :allow_membership_requests, - :membership_request_template, - ] + def group_params(automatic: false) + permitted_params = + if automatic + %i{ + visibility_level + mentionable_level + messageable_level + default_notification_level + } + else + 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 - permitted_params.push(:name) - end + if current_user.admin + default_params.push(:name) + end + + default_params + end params.require(:group).permit(*permitted_params) end diff --git a/spec/requests/groups_controller_spec.rb b/spec/requests/groups_controller_spec.rb index 5243092d817..c0d5fe11151 100644 --- a/spec/requests/groups_controller_spec.rb +++ b/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' } }