DEV: Make groups/new extensible by plugins (#7642)

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

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 03f5755..f2c6ca7 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 @@
     </label>
   </div>
 
+  {{plugin-outlet name="groups-form-membership-below-automatic"
+                  args=(hash model=model)}}
+
   <div class="control-group">
     <label class="control-label">{{i18n "admin.groups.manage.membership.trust_level"}}</label>
     <label for="grant_trust_level">{{i18n 'admin.groups.manage.membership.trust_levels_title'}}</label>
diff --git a/app/assets/javascripts/discourse/templates/group-index.hbs b/app/assets/javascripts/discourse/templates/group-index.hbs
index 583e827..397cf78 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 --}}
             </td>
           </tr>
         {{/each}}
diff --git a/app/controllers/admin/groups_controller.rb b/app/controllers/admin/groups_controller.rb
index 8281655..371f7a2 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 a9530d1..0a467fb 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 bb0adf7..88d37bb 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 7059844..dea2adb 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 d641fb1..c9e9422 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 7284da5..9125334 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

[... diff too long, it was truncated ...]

GitHub sha: c3a38d23

1 Like