Fixes #3648 - high organization user count relates to bad performance.

app/assets/javascripts/app/controllers/organization_profile.coffee

@@ -116,6 +116,7 @@ class ActionRow extends App.ControllerObserverActionRow
     ]
 
 class Object extends App.ControllerObserver
+  memberLimit: 10
   model: 'Organization'
   observe:
     member_ids: true
@@ -130,9 +131,36 @@ class Object extends App.ControllerObserver
     image_source: true
 
   events:
+    'click .js-showMoreMembers': 'showMoreMembers'
     'focusout [contenteditable]': 'update'
 
+  showMoreMembers: (e) ->
+    @preventDefaultAndStopPropagation(e)
+    @memberLimit = (parseInt(@memberLimit / 100) + 1) * 100
+    @renderMembers()
+
+  renderMembers: ->
+    elLocal = @el
+    @organization.members(0, @memberLimit, (users) ->
+      members = []
+      for user in users
+        el = $('<div></div>')
+        new Member(
+          object_id: user.id
+          el: el
+        )
+        members.push el
+      elLocal.find('.js-userList').html(members)
+    )
+
+    if @organization.member_ids.length < @memberLimit
+      @el.find('.js-showMoreMembers').parent().addClass('hidden')
+    else
+      @el.find('.js-showMoreMembers').parent().removeClass('hidden')
+
   render: (organization) =>
+    if organization
+      @organization = organization
 
     # update taskbar with new meta data
     App.TaskManager.touch(@taskKey)
@@ -144,20 +172,24 @@ class Object extends App.ControllerObserver
       # check if value for _id exists
       name    = attributeName
       nameNew = name.substr(0, name.length - 3)
-      if nameNew of organization
+      if nameNew of @organization
         name = nameNew
 
       # add to show if value exists
-      if (organization[name] || attributeConfig.tag is 'richtext') && attributeConfig.shown
+      if (@organization[name] || attributeConfig.tag is 'richtext') && attributeConfig.shown
 
         # do not show firstname and lastname / already show via diplayName()
         if name isnt 'name'
           organizationData.push attributeConfig
 
-    @html App.view('organization_profile/object')(
+    elLocal = $(App.view('organization_profile/object')(
-      organization:     organization
+      organization:     @organization
       organizationData: organizationData
-    )
+    ))
+
+    @html elLocal
+
+    @renderMembers()
 
     @$('[contenteditable]').ce({
       mode:      'textonly'
@@ -165,17 +197,6 @@ class Object extends App.ControllerObserver
       maxlength: 250
     })
 
-    # show members
-    members = []
-    for userId in organization.member_ids
-      el = $('<div></div>')
-      new Member(
-        object_id: userId
-        el: el
-      )
-      members.push el
-    @$('.js-userList').html(members)
-
   update: (e) =>
     name  = $(e.target).attr('data-name')
     value = $(e.target).html()

app/assets/javascripts/app/controllers/widget/organization.coffee

@@ -1,6 +1,8 @@
 class App.WidgetOrganization extends App.Controller
+  memberLimit: 10
 
   events:
+    'click .js-showMoreMembers': 'showMoreMembers'
     'focusout [contenteditable]': 'update'
 
   constructor: ->
@@ -12,9 +14,35 @@ class App.WidgetOrganization extends App.Controller
   release: =>
     App.Organization.unsubscribe(@subscribeId)
 
+  showMoreMembers: (e) ->
+    @preventDefaultAndStopPropagation(e)
+    @memberLimit = (parseInt(@memberLimit / 100) + 1) * 100
+    @renderMembers()
+
+  renderMembers: ->
+    elLocal = @el
+    @organization.members(0, @memberLimit, (users) ->
+      members = []
+      for user in users
+        el = $('<div></div>')
+        new Member(
+          object_id: user.id
+          el: el
+        )
+        members.push el
+      elLocal.find('.js-userList').html(members)
+    )
+
+    if @organization.member_ids.length < @memberLimit
+      @el.find('.js-showMoreMembers').parent().addClass('hidden')
+    else
+      @el.find('.js-showMoreMembers').parent().removeClass('hidden')
+
   render: (organization) =>
-    if !organization
+    if organization
-      organization = @u
+      @organization = organization
+    else if !@organization
+      @organization = @u
 
     # get display data
     organizationData = []
@@ -23,7 +51,7 @@ class App.WidgetOrganization extends App.Controller
       # check if value for _id exists
       name    = attributeName
       nameNew = name.substr( 0, name.length - 3 )
-      if nameNew of organization
+      if nameNew of @organization
         name = nameNew
 
       # do not show name since it's already shown via diplayName()
@@ -36,25 +64,21 @@ class App.WidgetOrganization extends App.Controller
       # Always show for these two conditions:
       # 1. the attribute exists and is not empty
       # 2. it is a richtext note field
-      continue if ( !organization[name]? || organization[name] is '' ) && attributeConfig.tag isnt 'richtext'
+      continue if ( !@organization[name]? || @organization[name] is '' ) && attributeConfig.tag isnt 'richtext'
 
       # add to show if all checks passed
       organizationData.push attributeConfig
 
     # insert userData
     elLocal = $(App.view('widget/organization')(
-      organization:     organization
+      organization:     @organization
       organizationData: organizationData
     ))
 
-    for user in organization.members
-      new User(
-        object_id: user.id
-        el: elLocal.find('div.userList-row[data-id=' + user.id + ']')
-      )
-
     @html elLocal
 
+    @renderMembers()
+
     @$('[contenteditable]').ce(
       mode:      'textonly'
       multiline: true
@@ -71,7 +95,7 @@ class App.WidgetOrganization extends App.Controller
       org.updateAttributes(data)
       @log 'notice', 'update', name, value, org
 
-class User extends App.ControllerObserver
+class Member extends App.ControllerObserver
   @extend App.PopoverProvidable
   @registerPopovers 'User'
 
@@ -80,6 +104,7 @@ class User extends App.ControllerObserver
     firstname: true
     lastname: true
     image: true
+    active: true
 
   render: (user) =>
     @html App.view('organization_profile/member')(

app/assets/javascripts/app/lib/app_post/popover_provider/organization_popover_provider.coffee

@@ -7,4 +7,20 @@ class Organization extends App.SingleObjectPopoverProvider
   displayTitleUsing: (object) ->
     object.name
 
+  buildHtmlContent: (params) ->
+    html = super
+
+    params.object.members(0, 10, (users) ->
+      members = []
+      for user in users
+        el = $('<div class="person"></div>')
+        if user.active is false
+          el.addClass('is-inactive')
+        el.append(user.displayName())
+        members.push el
+      html.find('.js-userList').html(members)
+    )
+
+    html
+
 App.PopoverProvider.registerProvider('Organization', Organization)

app/assets/javascripts/app/models/organization.coffee

@@ -29,16 +29,36 @@ Using **Organisations** you can **group** customers. This has among others two i
   icon: ->
     'organization'
 
-  @_fillUp: (data) ->
+  members: (offset, limit, callback) ->
+    member_ids         = @member_ids.slice(offset, limit)
+    missing_member_ids = _.filter(member_ids, (id) -> !App.User.findNative(id))
 
-    # add users of organization
+    userResult = ->
-    if data['member_ids']
+      users = []
-      data['members'] = []
+      for user_id in member_ids
-      for user_id in data['member_ids']
+        user = App.User.find(user_id)
-        if App.User.exists(user_id)
+        continue if !user
-          user = App.User.findNative(user_id)
+        users.push(user)
-          data['members'].push user
+      return users
-    data
+
+    return callback(userResult()) if missing_member_ids.length < 1
+
+    App.Ajax.request(
+      type: 'POST'
+      url: "#{@constructor.apiPath}/users/search"
+      data: JSON.stringify(
+        query: '*'
+        ids: missing_member_ids
+        limit: limit
+        full:  true
+      )
+      processData: true,
+      success: (data, status, xhr) ->
+        App.Collection.loadAssets(data.assets)
+        callback(userResult())
+      error: (data, status) ->
+        callback([])
+    )
 
   searchResultAttributes: ->
     classList = ['organization', 'organization-popover' ]

app/assets/javascripts/app/views/organization_profile/object.jst.eco

@@ -17,9 +17,12 @@
     <% end %>
   </div>
 </div>
-<% if @organization.members: %>
+<% if @organization.member_ids: %>
 <div class="profile-section profile-memberSection">
   <label><%- @T('Members') %></label>
   <div class="userList js-userList"></div>
+  <p class="hidden">
+    <a href="#" class="js-showMoreMembers"><%- @T('show more') %></a>
+  <p>
 </div>
 <% end %>

app/assets/javascripts/app/views/popover/organization.jst.eco

@@ -1,11 +1,9 @@
 <%- @V('popover/single_object_generic', object: @object, attributes: @attributes) %>
 
-<% if  @object.members: %>
+<% if  @object.member_ids: %>
   <hr>
   <div class="popover-block">
     <label><%- @T('Members') %></label>
-    <% for user in @object.members: %>
+    <div class="userList js-userList"></div>
-      <div class="person<% if user.active is false: %> is-inactive<% end %>"><%= user.displayName() %></div>
-    <% end %>
   </div>
 <% end %>

app/assets/javascripts/app/views/widget/organization.jst.eco

@@ -25,14 +25,13 @@
   <% end %>
 <% end %>
 
-<% if @organization.members: %>
+<% if @organization.member_ids: %>
 <hr>
 <div class="sidebar-block">
   <label><%- @T('Members') %></label>
-  <div class="userList">
+  <div class="userList js-userList"></div>
-<% for user in @organization.members: %>
+  <p class="hidden">
-    <div class="userList-row" data-id="<%- user.id %>"></div>
+    <a href="#" class="js-showMoreMembers"><%- @T('show more') %></a>
-<% end %>
+  <p>
-  </div>
 </div>
 <% end %>

app/controllers/users_controller.rb

@@ -217,11 +217,12 @@ class UsersController < ApplicationController
   #                   The requester has to be in the role 'Admin' or 'Agent' to
   #                   be able to search for User records.
   #
-  # @parameter        query            [String]                             The search query.
+  # @parameter        query             [String]                             The search query.
-  # @parameter        limit            [Integer]                            The limit of search results.
+  # @parameter        limit             [Integer]                            The limit of search results.
-  # @parameter        role_ids(multi)  [Array<String>]                      A list of Role identifiers to which the Users have to be allocated to.
+  # @parameter        ids(multi)        [Array<Integer>]                     A list of User IDs which should be returned
-  # @parameter        group_ids(multi) [Hash<String=>String,Array<String>>] A list of Group identifiers to which the Users have to be allocated to.
+  # @parameter        role_ids(multi)   [Array<String>]                      A list of Role identifiers to which the Users have to be allocated to.
-  # @parameter        full             [Boolean]                            Defines if the result should be
+  # @parameter        group_ids(multi)  [Hash<String=>String,Array<String>>] A list of Group identifiers to which the Users have to be allocated to.
+  # @parameter        full              [Boolean]                            Defines if the result should be
   #                                                                         true: { user_ids => [1,2,...], assets => {...} }
   #                                                                         or false: [{:id => user.id, :label => "firstname lastname <email>", :value => "firstname lastname <email>"},...].
   #
@@ -255,7 +256,7 @@ class UsersController < ApplicationController
       order_by:     params[:order_by],
       current_user: current_user,
     }
-    %i[role_ids group_ids permissions].each do |key|
+    %i[ids role_ids group_ids permissions].each do |key|
       next if params[key].blank?
 
       query_params[key] = params[key]

app/models/organization/assets.rb

@@ -40,11 +40,10 @@ returns
       app_model_user = User.to_app_model
       if local_attributes['member_ids'].present?
 
-        # feature used for different purpose; do limit references
+        # only provide assets for the first 10 organization users
-        if local_attributes['member_ids'].count > 100
+        # rest will be loaded optionally by the frontend
-          local_attributes['member_ids'] = local_attributes['member_ids'].sort[0, 100]
+        local_attributes['member_ids'] = local_attributes['member_ids'].sort
-        end
+        local_attributes['member_ids'][0, 10].each do |local_user_id|
-        local_attributes['member_ids'].each do |local_user_id|
           next if data[ app_model_user ] && data[ app_model_user ][ local_user_id ]
 
           user = User.lookup(id: local_user_id)

app/models/user/search.rb

@@ -114,16 +114,20 @@ returns
             }
             query_extension['bool']['must'].push access_condition
           end
+
+          user_ids = []
           if params[:group_ids].present?
-            query_extension['bool'] ||= {}
-            query_extension['bool']['must'] ||= []
-            user_ids = []
             params[:group_ids].each do |group_id, access|
               user_ids |= User.group_access(group_id.to_i, access).pluck(:id)
             end
-
             return [] if user_ids.blank?
-
+          end
+          if params[:ids].present?
+            user_ids |= params[:ids].map(&:to_i)
+          end
+          if user_ids.present?
+            query_extension['bool'] ||= {}
+            query_extension['bool']['must'] ||= []
             query_extension['bool']['must'].push({ 'terms' => { '_id' => user_ids } })
           end
 
@@ -149,6 +153,10 @@ returns
         query.delete! '*'
 
         statement = User
+        if params[:ids].present?
+          statement = statement.where(id: params[:ids])
+        end
+
         if params[:role_ids]
           statement = statement.joins(:roles).where('roles.id' => params[:role_ids])
         end

spec/requests/user_spec.rb

@@ -1507,6 +1507,50 @@ RSpec.describe 'User', type: :request do
     end
   end
 
+  describe 'GET /api/v1/users/search, checks usage of the ids parameter', authenticated_as: :agent do
+    let(:agent) { create(:agent) }
+
+    let(:search_agents) { create_list(:agent, 3, firstname: 'Nick') }
+
+    shared_examples 'ids requests' do
+
+      before do
+        post '/api/v1/users/search', params: { query: 'Nick', ids: search_ids, sort_by: ['created_at'], order_by: ['ASC'] }, as: :json
+      end
+
+      shared_examples 'result check' do
+
+        it 'returns only agents matching search parameter ids' do
+          expect(json_response.map { |row| row['id'] }).to eq(search_ids)
+        end
+      end
+
+      context 'when searching for first two agents' do
+        let(:search_ids) { search_agents.first(2).map(&:id) }
+
+        include_examples 'result check'
+      end
+
+      context 'when searching for last two agents' do
+        let(:search_ids) { search_agents.last(2).map(&:id) }
+
+        include_examples 'result check'
+      end
+    end
+
+    context 'with elasticsearch', searchindex: true do
+      include_examples 'ids requests' do
+        before do
+          configure_elasticsearch(required: true, rebuild: true)
+        end
+      end
+    end
+
+    context 'without elasticsearch' do
+      include_examples 'ids requests'
+    end
+  end
+
   describe 'PUT /api/v1/users/unlock/{id}' do
     let(:admin) { create(:admin) }
     let(:agent) { create(:agent) }

spec/system/organization_profile_spec.rb

@@ -0,0 +1,24 @@
+# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/
+
+require 'rails_helper'
+
+RSpec.describe 'Organization Profile', type: :system do
+
+  context 'members section' do
+    let(:organization) { create(:organization) }
+    let(:members) { organization.members.order(id: :asc) }
+
+    before do
+      create_list(:customer, 50, organization: organization)
+      visit "#organization/profile/#{organization.id}"
+    end
+
+    it 'shows first 10 members and loads more on demand' do
+      expect(page).to have_text(members[9].fullname)
+      expect(page).to have_no_text(members[10].fullname)
+
+      click '.js-showMoreMembers'
+      expect(page).to have_text(members[10].fullname)
+    end
+  end
+end

spec/system/search_spec.rb

@@ -17,6 +17,28 @@ RSpec.describe 'Search', type: :system, searchindex: true do
     end
   end
 
+  context 'Organization members', authenticated_as: :authenticate do
+    let(:organization) { create(:organization) }
+    let(:members) { organization.members.order(id: :asc) }
+
+    def authenticate
+      create_list(:customer, 50, organization: organization)
+      true
+    end
+
+    before do
+      sleep 3 # wait for popover killer to pass
+      fill_in id: 'global-search', with: organization.name.to_s
+    end
+
+    it 'shows only first 10 members' do
+      expect(page).to have_text(organization.name)
+      popover_on_hover(first('a.nav-tab.organization'))
+      expect(page).to have_text(members[9].fullname, wait: 30)
+      expect(page).to have_no_text(members[10].fullname)
+    end
+  end
+
   context 'inactive user and organizations' do
     before do
       create(:organization, name: 'Example Inc.', active: true)

spec/system/ticket/zoom_spec.rb

@@ -1682,4 +1682,28 @@ RSpec.describe 'Ticket zoom', type: :system do
       expect(page).to have_text(ticket_closed.title, wait: 20)
     end
   end
+
+  context 'Sidebar - Organization' do
+    let(:organization) { create(:organization) }
+
+    context 'members section' do
+
+      let(:customers) { create_list(:customer, 50, organization: organization) }
+      let(:ticket) { create(:ticket, group: Group.find_by(name: 'Users'), customer: customers.first) }
+      let(:members) { organization.members.order(id: :asc) }
+
+      before do
+        visit "#ticket/zoom/#{ticket.id}"
+        click '.tabsSidebar-tab[data-tab=organization]'
+      end
+
+      it 'shows first 10 members and loads more on demand' do
+        expect(page).to have_text(members[9].fullname)
+        expect(page).to have_no_text(members[10].fullname)
+
+        click '.js-showMoreMembers'
+        expect(page).to have_text(members[10].fullname)
+      end
+    end
+  end
 end