From c3a38d23040f7b11dee5f74c43cbede607b203bc Mon Sep 17 00:00:00 2001 From: Roman Rizzi Date: Wed, 5 Jun 2019 23:05:33 -0300 Subject: [PATCH] DEV: Make groups/new extensible by plugins (#7642) * Expose a new plugin outlet. Pass group model to the group-member-dropdown so it can be accessed by plugins * Added controller tests for group custom fields. update custom fields when updating a group --- .../groups-form-membership-fields.hbs | 3 ++ .../discourse/templates/group-index.hbs | 4 +- app/controllers/admin/groups_controller.rb | 8 +++- app/controllers/groups_controller.rb | 3 ++ app/models/group.rb | 15 ++++++ lib/plugin/instance.rb | 6 +++ spec/requests/admin/groups_controller_spec.rb | 46 ++++++++++++++++++- spec/requests/groups_controller_spec.rb | 37 ++++++++++++++- 8 files changed, 116 insertions(+), 6 deletions(-) diff --git a/app/assets/javascripts/discourse/templates/components/groups-form-membership-fields.hbs b/app/assets/javascripts/discourse/templates/components/groups-form-membership-fields.hbs index 03f5755d8f9..f2c6ca7593e 100644 --- a/app/assets/javascripts/discourse/templates/components/groups-form-membership-fields.hbs +++ b/app/assets/javascripts/discourse/templates/components/groups-form-membership-fields.hbs @@ -27,6 +27,9 @@ + {{plugin-outlet name="groups-form-membership-below-automatic" + args=(hash model=model)}} +
diff --git a/app/assets/javascripts/discourse/templates/group-index.hbs b/app/assets/javascripts/discourse/templates/group-index.hbs index 583e827abe1..397cf7808e7 100644 --- a/app/assets/javascripts/discourse/templates/group-index.hbs +++ b/app/assets/javascripts/discourse/templates/group-index.hbs @@ -64,8 +64,10 @@ removeMember=(action "removeMember") makeOwner=(action "makeOwner") removeOwner=(action "removeOwner") - member=m}} + member=m + group=model}} {{/if}} + {{!-- group parameter is used by plugins --}} {{/each}} diff --git a/app/controllers/admin/groups_controller.rb b/app/controllers/admin/groups_controller.rb index 8281655910d..371f7a2bc0f 100644 --- a/app/controllers/admin/groups_controller.rb +++ b/app/controllers/admin/groups_controller.rb @@ -130,7 +130,7 @@ class Admin::GroupsController < Admin::AdminController private def group_params - params.require(:group).permit( + permitted = [ :name, :mentionable_level, :messageable_level, @@ -153,6 +153,10 @@ class Admin::GroupsController < Admin::AdminController :membership_request_template, :owner_usernames, :usernames - ) + ] + custom_fields = Group.editable_group_custom_fields + permitted << { custom_fields: custom_fields } unless custom_fields.blank? + + params.require(:group).permit(permitted) end end diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index a9530d1aac0..0a467fba42b 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -545,6 +545,9 @@ class GroupsController < ApplicationController :automatic_membership_email_domains, :automatic_membership_retroactive ]) + + custom_fields = Group.editable_group_custom_fields + default_params << { custom_fields: custom_fields } unless custom_fields.blank? end default_params diff --git a/app/models/group.rb b/app/models/group.rb index bb0adf78e43..88d37bb58aa 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -189,6 +189,21 @@ class Group < ActiveRecord::Base levels end + def self.plugin_editable_group_custom_fields + @plugin_editable_group_custom_fields ||= {} + end + + def self.register_plugin_editable_group_custom_field(custom_field_name, plugin) + plugin_editable_group_custom_fields[custom_field_name] = plugin + end + + def self.editable_group_custom_fields + plugin_editable_group_custom_fields.reduce([]) do |fields, (k, v)| + next(fields) unless v.enabled? + fields << k + end.uniq + end + def downcase_incoming_email self.incoming_email = (incoming_email || "").strip.downcase.presence end diff --git a/lib/plugin/instance.rb b/lib/plugin/instance.rb index 7059844987a..dea2adbb878 100644 --- a/lib/plugin/instance.rb +++ b/lib/plugin/instance.rb @@ -146,6 +146,12 @@ class Plugin::Instance end end + def register_editable_group_custom_field(field) + reloadable_patch do |plugin| + ::Group.register_plugin_editable_group_custom_field(field, plugin) # plugin.enabled? is checked at runtime + end + end + def custom_avatar_column(column) reloadable_patch do |plugin| AvatarLookup.lookup_columns << column diff --git a/spec/requests/admin/groups_controller_spec.rb b/spec/requests/admin/groups_controller_spec.rb index d641fb1d77b..c9e94223151 100644 --- a/spec/requests/admin/groups_controller_spec.rb +++ b/spec/requests/admin/groups_controller_spec.rb @@ -12,8 +12,8 @@ RSpec.describe Admin::GroupsController do end describe '#create' do - it 'should work' do - post "/admin/groups.json", params: { + let(:group_params) do + { group: { name: 'testing', usernames: [admin.username, user.username].join(","), @@ -22,6 +22,10 @@ RSpec.describe Admin::GroupsController do membership_request_template: 'Testing', } } + end + + it 'should work' do + post "/admin/groups.json", params: group_params expect(response.status).to eq(200) @@ -32,6 +36,44 @@ RSpec.describe Admin::GroupsController do expect(group.allow_membership_requests).to eq(true) expect(group.membership_request_template).to eq('Testing') end + + context "custom_fields" do + before do + plugin = Plugin::Instance.new + plugin.register_editable_group_custom_field :test + end + + after do + Group.plugin_editable_group_custom_fields.clear + end + + it "only updates allowed user fields" do + params = group_params + params[:group].merge!(custom_fields: { test: :hello1, test2: :hello2 }) + + post "/admin/groups.json", params: params + + group = Group.last + + expect(response.status).to eq(200) + expect(group.custom_fields['test']).to eq('hello1') + expect(group.custom_fields['test2']).to be_blank + end + + it "is secure when there are no registered editable fields" do + Group.plugin_editable_group_custom_fields.clear + params = group_params + params[:group].merge!(custom_fields: { test: :hello1, test2: :hello2 }) + + post "/admin/groups.json", params: params + + group = Group.last + + expect(response.status).to eq(200) + expect(group.custom_fields['test']).to be_blank + expect(group.custom_fields['test2']).to be_blank + end + end end describe '#add_owners' do diff --git a/spec/requests/groups_controller_spec.rb b/spec/requests/groups_controller_spec.rb index 7284da58d3e..912533484d8 100644 --- a/spec/requests/groups_controller_spec.rb +++ b/spec/requests/groups_controller_spec.rb @@ -469,7 +469,7 @@ describe GroupsController do end describe '#update' do - let(:group) do + let!(:group) do Fabricate(:group, name: 'test', users: [user], @@ -478,6 +478,41 @@ describe GroupsController do ) end + context "custom_fields" do + before do + user.update!(admin: true) + sign_in(user) + plugin = Plugin::Instance.new + plugin.register_editable_group_custom_field :test + @group = Fabricate(:group) + end + + after do + Group.plugin_editable_group_custom_fields.clear + end + + it "only updates allowed user fields" do + put "/groups/#{@group.id}.json", params: { group: { custom_fields: { test: :hello1, test2: :hello2 } } } + + @group.reload + + expect(response.status).to eq(200) + expect(@group.custom_fields['test']).to eq('hello1') + expect(@group.custom_fields['test2']).to be_blank + end + + it "is secure when there are no registered editable fields" do + Group.plugin_editable_group_custom_fields.clear + put "/groups/#{@group.id}.json", params: { group: { custom_fields: { test: :hello1, test2: :hello2 } } } + + @group.reload + + expect(response.status).to eq(200) + expect(@group.custom_fields['test']).to be_blank + expect(@group.custom_fields['test2']).to be_blank + end + end + context "when user is group owner" do before do group.add_owner(user)