Browse Source

Follow up 34ec90bd738283ee894c3059039e7868a4049a17 - Fixes #4243 - Zammad suffers from serious performance degrade on incorrect role configurations.

Rolf Schmidt 2 years ago
parent
commit
c85b344428

+ 12 - 0
app/assets/javascripts/app/controllers/_ui_element/permission.coffee

@@ -25,6 +25,18 @@ class App.UiElement.permission extends App.UiElement.ApplicationUiElement
       groupAccesses: App.Group.accesses()
     ) )
 
+    item.find('div.js-groupList input[type="checkbox"]').on('change', (e) ->
+      checked = item.find('div.js-groupList input[type="checkbox"]:checked').length > 0 ? true : false
+      parentCheckbox = item.find('div.js-groupList input[type="checkbox"]').parents('.js-subPermissionList').find('input[type="checkbox"][data-permission-name]')
+
+      parentCheckbox.prop('checked', checked)
+    )
+
+    item.find('div.js-groupList input[type="checkbox"]').parents('.js-subPermissionList').find('input[type="checkbox"][data-permission-name]').on('change', (e) ->
+      return if $(e.currentTarget).prop('checked')
+      item.find('div.js-groupList input[type="checkbox"]:checked').prop('checked', false)
+    )
+
     # show/hide trees
     item.find('[name=permission_ids]').on('change', (e) =>
       @checkUncheck($(e.currentTarget), permissions, item)

+ 1 - 1
app/assets/javascripts/app/models/permission.coffee

@@ -1,4 +1,4 @@
 class App.Permission extends App.Model
-  @configure 'Permission', 'name', 'note', 'active'
+  @configure 'Permission', 'name', 'note', 'active', 'preferences'
   @extend Spine.Model.Ajax
   @url: @apiPath + '/permissions'

+ 1 - 8
app/models/concerns/has_groups.rb

@@ -317,16 +317,9 @@ module HasGroups
       access   = Array(access).map(&:to_sym) | [:full]
 
       # check direct access
-      instances = joins(group_through.name)
+      instances = Permission.join_with(self, 'ticket.agent').joins(group_through.name)
                   .where(group_through.table_name => { group_id: group_id, access: access }, active: true)
 
-      if method_defined?(:permissions?)
-        permissions = Permission.with_parents('ticket.agent')
-        instances = instances
-                    .joins(roles: :permissions)
-                    .where(roles: { active: true }, permissions: { name: permissions, active: true })
-      end
-
       # check indirect access through roles if possible
       return instances if !respond_to?(:role_access)
 

+ 2 - 1
app/models/concerns/has_roles.rb

@@ -69,7 +69,8 @@ module HasRoles
 
       role_ids   = RoleGroup.eager_load(:role).where(group_id: group_id, access: access, roles: { active: true }).pluck(:role_id)
       join_table = reflect_on_association(:roles).join_table
-      joins(:roles).where(active: true, join_table => { role_id: role_ids }).distinct.select(&:groups_access_permission?)
+
+      Permission.join_with(self, 'ticket.agent').joins(:roles).where(active: true, join_table => { role_id: role_ids }).distinct
     end
 
     # Lists IDs of instances having the given access(es) to the given Group through Roles.

+ 8 - 0
app/models/permission.rb

@@ -39,4 +39,12 @@ returns
     name
   end
 
+  def self.join_with(object, permissions)
+    return object if !object.method_defined?(:permissions?)
+
+    permissions = with_parents(permissions)
+
+    object.joins(roles: :permissions)
+                .where(roles: { active: true }, permissions: { name: permissions, active: true })
+  end
 end

File diff suppressed because it is too large
+ 2 - 0
public/assets/tests/qunit/form_extended.js


+ 1 - 1
spec/models/concerns/has_roles_examples.rb

@@ -4,7 +4,7 @@ RSpec.shared_examples 'HasRoles' do |group_access_factory:|
   context 'role' do
     subject { create(group_access_factory) }
 
-    let(:role)           { create(:role) }
+    let(:role)           { create(:agent_role) }
     let(:group_instance) { create(:group) }
     let(:group_role)     { create(:group) }
     let(:group_inactive) { create(:group, active: false) }

+ 13 - 0
spec/models/permission_spec.rb

@@ -22,4 +22,17 @@ RSpec.describe Permission, type: :model do
       end
     end
   end
+
+  describe '.join_with' do
+    let!(:agents)    { create_list(:agent, 5) }
+    let!(:customers) { create_list(:customer, 5) }
+
+    it 'does include agents' do
+      expect(described_class.join_with(User, 'ticket.agent')).to include(*agents)
+    end
+
+    it 'does exclude customers' do
+      expect(described_class.join_with(User, 'ticket.agent')).not_to include(*customers)
+    end
+  end
 end

Some files were not shown because too many files changed in this diff