diff --git a/app/assets/javascripts/app/controllers/organization_profile.coffee b/app/assets/javascripts/app/controllers/organization_profile.coffee index b4c5f59e3..cb4caaf0f 100644 --- a/app/assets/javascripts/app/controllers/organization_profile.coffee +++ b/app/assets/javascripts/app/controllers/organization_profile.coffee @@ -112,6 +112,8 @@ class ActionRow extends App.ObserverActionRow class Object extends App.ObserverController model: 'Organization' + observe: + member_ids: true observeNot: cid: true created_at: true @@ -196,6 +198,7 @@ class Member extends App.ObserverController login: true email: true active: true + image: true globalRerender: false render: (user) => diff --git a/app/assets/javascripts/app/controllers/user_profile.coffee b/app/assets/javascripts/app/controllers/user_profile.coffee index 7653d6a2a..9ea7773cd 100644 --- a/app/assets/javascripts/app/controllers/user_profile.coffee +++ b/app/assets/javascripts/app/controllers/user_profile.coffee @@ -43,15 +43,9 @@ class App.UserProfile extends App.Controller new User( object_id: user.id - el: elLocal.find('.js-name') + el: elLocal.find('.js-profileName') ) - if user.organization_id - new Organization( - object_id: user.organization_id - el: elLocal.find('.js-organization') - ) - new Object( el: elLocal.find('.js-object-container') object_id: user.id @@ -206,9 +200,19 @@ class User extends App.ObserverController observe: firstname: true lastname: true + organization_id: true + image: true render: (user) => - @html App.Utils.htmlEscape(user.displayName()) + if user.organization_id + new Organization( + object_id: user.organization_id + el: @el.siblings('.js-organization') + ) + + @html App.view('user_profile/name')( + user: user + ) class Router extends App.ControllerPermanent requiredPermission: 'ticket.agent' diff --git a/app/assets/javascripts/app/controllers/widget/organization.coffee b/app/assets/javascripts/app/controllers/widget/organization.coffee index 9eb2ac433..6031d45bd 100644 --- a/app/assets/javascripts/app/controllers/widget/organization.coffee +++ b/app/assets/javascripts/app/controllers/widget/organization.coffee @@ -1,6 +1,4 @@ class App.WidgetOrganization extends App.Controller - @extend App.PopoverProvidable - @registerPopovers 'User' events: 'focusout [contenteditable]': 'update' @@ -44,10 +42,18 @@ class App.WidgetOrganization extends App.Controller organizationData.push attributeConfig # insert userData - @html App.view('widget/organization')( + elLocal = $(App.view('widget/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 @$('[contenteditable]').ce( mode: 'textonly' @@ -55,8 +61,6 @@ class App.WidgetOrganization extends App.Controller maxlength: 250 ) - @renderPopovers() - update: (e) => name = $(e.target).attr('data-name') value = $(e.target).html() @@ -66,3 +70,21 @@ class App.WidgetOrganization extends App.Controller data[name] = value org.updateAttributes(data) @log 'notice', 'update', name, value, org + +class User extends App.ObserverController + @extend App.PopoverProvidable + @registerPopovers 'User' + + model: 'User' + observe: + firstname: true + lastname: true + image: true + + render: (user) => + @html App.view('organization_profile/member')( + user: user + el: @el, + ) + + @renderPopovers() diff --git a/app/assets/javascripts/app/views/user_profile/index.jst.eco b/app/assets/javascripts/app/views/user_profile/index.jst.eco index 601eca945..2155f1e76 100644 --- a/app/assets/javascripts/app/views/user_profile/index.jst.eco +++ b/app/assets/javascripts/app/views/user_profile/index.jst.eco @@ -2,8 +2,7 @@
- <%- @user.avatar("80") %> -

+
<% if @user.organization: %>
<% end %> diff --git a/app/assets/javascripts/app/views/user_profile/name.jst.eco b/app/assets/javascripts/app/views/user_profile/name.jst.eco new file mode 100644 index 000000000..2a179bca3 --- /dev/null +++ b/app/assets/javascripts/app/views/user_profile/name.jst.eco @@ -0,0 +1,2 @@ +<%- @user.avatar("80") %> +

<%= @user.displayName() %>

diff --git a/app/assets/javascripts/app/views/widget/organization.jst.eco b/app/assets/javascripts/app/views/widget/organization.jst.eco index 2b5d2daf9..1e315d651 100644 --- a/app/assets/javascripts/app/views/widget/organization.jst.eco +++ b/app/assets/javascripts/app/views/widget/organization.jst.eco @@ -27,13 +27,8 @@
<% for user in @organization.members: %> -
- <%- user.avatar("40") %> - - <%= user.displayName() %> - -
+
<% end %>
-<% end %> \ No newline at end of file +<% end %> diff --git a/app/jobs/search_index_job.rb b/app/jobs/search_index_job.rb index f6ee8cddb..26ed95272 100644 --- a/app/jobs/search_index_job.rb +++ b/app/jobs/search_index_job.rb @@ -6,11 +6,11 @@ class SearchIndexJob < ApplicationJob } def lock_key - # "SearchIndexJob/User/42" - "#{self.class.name}/#{arguments[0]}/#{arguments[1]}" + # "SearchIndexJob/User/42/true" + "#{self.class.name}/#{arguments[0]}/#{arguments[1]}/#{arguments[2]}" end - def perform(object, o_id) + def perform(object, o_id, update_associations = true) @object = object @o_id = o_id @@ -18,6 +18,11 @@ class SearchIndexJob < ApplicationJob return if !exists?(record) record.search_index_update_backend + + return if !update_associations + + record.search_index_update_associations_delta + record.search_index_update_associations_full end private diff --git a/app/models/application_model/can_lookup_search_index_attributes.rb b/app/models/application_model/can_lookup_search_index_attributes.rb index 74748d403..80be68498 100644 --- a/app/models/application_model/can_lookup_search_index_attributes.rb +++ b/app/models/application_model/can_lookup_search_index_attributes.rb @@ -19,48 +19,147 @@ returns attributes = self.attributes self.attributes.each do |key, value| + attribute_name = key.to_s + + # ignore standard attribute if needed + if self.class.search_index_attribute_ignored?(attribute_name) + attributes.delete(attribute_name) + next + end + + # need value for reference data next if !value - # get attribute name - attribute_name = key.to_s - next if attribute_name[-3, 3] != '_id' + # check if we have a referenced object which we could include here + next if !search_index_attribute_method(attribute_name) - attribute_name = attribute_name[ 0, attribute_name.length - 3 ] + # get referenced attribute name + attribute_ref_name = self.class.search_index_attribute_ref_name(attribute_name) + next if !attribute_ref_name - # check if attribute method exists - next if !respond_to?(attribute_name) - - # check if method has own class - relation_class = send(attribute_name).class - next if !relation_class - - # lookup ref object - relation_model = relation_class.lookup(id: value) - next if !relation_model - - # get name of ref object - value = nil - if relation_model.respond_to?('search_index_data') - value = relation_model.send('search_index_data') - end - - if relation_model.respond_to?('name') - value = relation_model.send('name') - end + # ignore referenced attributes if needed + next if self.class.search_index_attribute_ignored?(attribute_ref_name) + # get referenced attribute value + value = search_index_value_by_attribute(attribute_name) next if !value # save name of ref object - attributes[ attribute_name ] = value - end - - ignored_attributes = self.class.instance_variable_get(:@search_index_attributes_ignored) || [] - return attributes if ignored_attributes.blank? - - ignored_attributes.each do |attribute| - attributes.delete(attribute.to_s) + attributes[ attribute_ref_name ] = value end attributes end + +=begin + +This function returns the relational search index value based on the attribute name. + + organization = Organization.find(1) + value = organization.search_index_value_by_attribute('organization_id') + +returns + + value = {"name"=>"Zammad Foundation"} + +=end + + def search_index_value_by_attribute(attribute_name = '') + + # get attribute name + relation_class = search_index_attribute_method(attribute_name) + return if !relation_class + + # lookup ref object + relation_model = relation_class.lookup(id: attributes[attribute_name]) + return if !relation_model + + relation_model.search_index_value + end + +=begin + +This function returns the relational search value. + + organization = Organization.find(1) + value = organization.search_index_value + +returns + + value = {"name"=>"Zammad Foundation"} + +=end + + def search_index_value + + # get name of ref object + value = nil + if respond_to?('search_index_data') + value = send('search_index_data') + return if value == true + elsif respond_to?('name') + value = send('name') + end + + value + end + +=begin + +This function returns the method for the relational search index attribute. + + method = Ticket.new.search_index_attribute_method('organization_id') + +returns + + method = Organization (class) + +=end + + def search_index_attribute_method(attribute_name = '') + return if attribute_name[-3, 3] != '_id' + + attribute_name = attribute_name[ 0, attribute_name.length - 3 ] + return if !respond_to?(attribute_name) + + send(attribute_name).class + end + + class_methods do + +=begin + +This function returns the relational search index attribute name for the given class. + + attribute_ref_name = Organization.search_index_attribute_ref_name('user_id') + +returns + + attribute_ref_name = 'user' + +=end + + def search_index_attribute_ref_name(attribute_name) + attribute_name[ 0, attribute_name.length - 3 ] + end + +=begin + +This function returns if a search index attribute should be ignored. + + ignored = Ticket.search_index_attribute_ignored?('organization_id') + +returns + + ignored = false + +=end + + def search_index_attribute_ignored?(attribute_name = '') + ignored_attributes = instance_variable_get(:@search_index_attributes_ignored) || [] + return if ignored_attributes.blank? + + ignored_attributes.include?(attribute_name.to_sym) + end + end end diff --git a/app/models/concerns/has_search_index_backend.rb b/app/models/concerns/has_search_index_backend.rb index bdf583b53..e65f60410 100644 --- a/app/models/concerns/has_search_index_backend.rb +++ b/app/models/concerns/has_search_index_backend.rb @@ -30,6 +30,86 @@ update search index, if configured - will be executed automatically =begin +update search index, if configured - will be executed automatically + + model = Organizations.find(123) + result = model.search_index_update_associations_full + +returns + + # Updates asscociation data for users and tickets of the organization in this example + result = true + +=end + + def search_index_update_associations_full + return if self.class.to_s != 'Organization' + + # reindex all organization tickets for the given organization id + # we can not use the delta function for this because of the excluded + # ticket article attachments. see explain in delta function + Ticket.select('id').where(organization_id: id).order(id: :desc).limit(10_000).pluck(:id).each do |ticket_id| + SearchIndexJob.perform_later('Ticket', ticket_id, false) + end + end + +=begin + +update search index, if configured - will be executed automatically + + model = Organizations.find(123) + result = model.search_index_update_associations_delta + +returns + + # Updates asscociation data for users and tickets of the organization in this example + result = true + +=end + + def search_index_update_associations_delta + + # start background job to transfer data to search index + return true if !SearchIndexBackend.enabled? + return if search_index_value.blank? + + Models.indexable.each do |local_object| + next if local_object == self.class + + # delta update of associations is only possible for + # objects which are not containing modifications of the source + # https://github.com/zammad/zammad/blob/264853dcbe4e53addaf0f8e6df3735ceddc9de63/lib/tasks/search_index_es.rake#L266 + # because of the exlusion of the article attachments for the ticket + # we dont have the attachment data available in the json store of the object. + # so the search index would lose the attachment information on the _update_by_query function + # https://www.elastic.co/guide/en/elasticsearch/reference/current/mapping-source-field.html + next if local_object.to_s == 'Ticket' + + local_object.new.attributes.each do |key, _value| + attribute_name = key.to_s + attribute_ref_name = local_object.search_index_attribute_ref_name(attribute_name) + attribute_class = local_object.reflect_on_association(attribute_ref_name)&.klass + + next if attribute_name.blank? + next if attribute_ref_name.blank? + next if attribute_class.blank? + next if attribute_class != self.class + + data = { + attribute_ref_name => search_index_value + } + where = { + attribute_name => id + } + SearchIndexBackend.update_by_query(local_object.to_s, data, where) + end + end + + true + end + +=begin + delete search index object, will be executed automatically model = Model.find(123) diff --git a/app/models/observer/organization/ref_object_touch.rb b/app/models/observer/organization/ref_object_touch.rb deleted file mode 100644 index dd618f9d8..000000000 --- a/app/models/observer/organization/ref_object_touch.rb +++ /dev/null @@ -1,43 +0,0 @@ -# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ - -class Observer::Organization::RefObjectTouch < ActiveRecord::Observer - observe 'organization' - - def after_create(record) - ref_object_touch(record) - end - - def after_update(record) - ref_object_touch(record) - end - - def after_destroy(record) - ref_object_touch(record) - end - - def ref_object_touch(record) - - # return if we run import mode - return true if Setting.get('import_mode') - - # feature used for different purpose; do not touch references - return true if User.where(organization_id: record.id).count > 100 - - # touch organizations tickets - Ticket.select('id').where(organization_id: record.id).pluck(:id).each do |ticket_id| - ticket = Ticket.find(ticket_id) - ticket.with_lock do - ticket.touch # rubocop:disable Rails/SkipsModelValidations - end - end - - # touch current members - User.select('id').where(organization_id: record.id).pluck(:id).each do |user_id| - user = User.find(user_id) - user.with_lock do - user.touch # rubocop:disable Rails/SkipsModelValidations - end - end - true - end -end diff --git a/app/models/observer/user/ref_object_touch.rb b/app/models/observer/user/ref_object_touch.rb index 68f323f11..f39c4d056 100644 --- a/app/models/observer/user/ref_object_touch.rb +++ b/app/models/observer/user/ref_object_touch.rb @@ -20,37 +20,22 @@ class Observer::User::RefObjectTouch < ActiveRecord::Observer # return if we run import mode return true if Setting.get('import_mode') - # touch old organization if changed - member_ids = [] organization_id_changed = record.saved_changes['organization_id'] - if organization_id_changed && organization_id_changed[0] != organization_id_changed[1] - if organization_id_changed[0] + return true if !organization_id_changed - # featrue used for different propose, do not touch references - if User.where(organization_id: organization_id_changed[0]).count < 100 - organization = Organization.find(organization_id_changed[0]) - organization.touch # rubocop:disable Rails/SkipsModelValidations - member_ids = organization.member_ids - end - end + return true if organization_id_changed[0] == organization_id_changed[1] + + # touch old organization + if organization_id_changed[0] + organization = Organization.find(organization_id_changed[0]) + organization.touch # rubocop:disable Rails/SkipsModelValidations end # touch new/current organization - if record.organization - - # featrue used for different propose, do not touch references - if User.where(organization_id: record.organization_id).count < 100 - record.organization.touch # rubocop:disable Rails/SkipsModelValidations - member_ids += record.organization.member_ids - end + if record&.organization + record.organization.touch # rubocop:disable Rails/SkipsModelValidations end - # touch old/current customer - member_ids.uniq.each do |user_id| - next if user_id == record.id - - User.find(user_id).touch # rubocop:disable Rails/SkipsModelValidations - end true end end diff --git a/config/application.rb b/config/application.rb index b66414fd5..cd446f493 100644 --- a/config/application.rb +++ b/config/application.rb @@ -48,7 +48,6 @@ module Zammad 'observer::_user::_ref_object_touch', 'observer::_user::_ticket_organization', 'observer::_user::_geo', - 'observer::_organization::_ref_object_touch', 'observer::_sla::_ticket_rebuild_escalation', 'observer::_transaction' diff --git a/lib/search_index_backend.rb b/lib/search_index_backend.rb index eb59fbf95..7b9ae3422 100644 --- a/lib/search_index_backend.rb +++ b/lib/search_index_backend.rb @@ -119,7 +119,7 @@ create/update/delete index def self.index(data) - url = build_url(data[:name], nil, false, false) + url = build_url(type: data[:name], with_pipeline: false, with_document_type: false) return if url.blank? if data[:action] && data[:action] == 'delete' @@ -139,7 +139,7 @@ add new object to search index def self.add(type, data) - url = build_url(type, data['id']) + url = build_url(type: type, object_id: data['id']) return if url.blank? make_request_and_validate(url, data: data, method: :post) @@ -147,6 +147,48 @@ add new object to search index =begin +This function updates specifc attributes of an index based on a query. + + data = { + organization: { + name: "Zammad Foundation" + } + } + where = { + organization_id: 1 + } + SearchIndexBackend.update_by_query('Ticket', data, where) + +=end + + def self.update_by_query(type, data, where) + return if data.blank? + return if where.blank? + + url = build_url(type: type, action: '_update_by_query', with_pipeline: false, with_document_type: false, url_params: { conflicts: 'proceed' }) + return if url.blank? + + script_list = [] + data.each do |key, _value| + script_list.push("ctx._source.#{key}=params.#{key}") + end + + data = { + script: { + lang: 'painless', + source: script_list.join(';'), + params: data, + }, + query: { + term: where, + }, + } + + make_request_and_validate(url, data: data, method: :post, read_timeout: 10.minutes) + end + +=begin + remove whole data from index SearchIndexBackend.remove('Ticket', 123) @@ -157,9 +199,9 @@ remove whole data from index def self.remove(type, o_id = nil) url = if o_id - build_url(type, o_id, false, true) + build_url(type: type, object_id: o_id, with_pipeline: false, with_document_type: true) else - build_url(type, o_id, false, false) + build_url(type: type, object_id: o_id, with_pipeline: false, with_document_type: false) end return if url.blank? @@ -237,11 +279,9 @@ remove whole data from index def self.search_by_index(query, index, options = {}) return [] if query.blank? - url = build_url + url = build_url(type: index, action: '_search', with_pipeline: false, with_document_type: true) return [] if url.blank? - url += build_search_url(index) - # real search condition condition = { 'query_string' => { @@ -389,11 +429,9 @@ example for aggregations within one year def self.selectors(index, selectors = nil, options = {}, aggs_interval = nil) raise 'no selectors given' if !selectors - url = build_url(nil, nil, false, false) + url = build_url(type: index, action: '_search', with_pipeline: false, with_document_type: true) return if url.blank? - url += build_search_url(index) - data = selector2query(selectors, options, aggs_interval) response = make_request(url, data: data) @@ -626,107 +664,92 @@ return true if backend is configured true end - def self.build_index_name(index) + def self.build_index_name(index = nil) local_index = "#{Setting.get('es_index')}_#{Rails.env}" + return local_index if index.blank? + return "#{local_index}/#{index}" if lower_equal_es56? "#{local_index}_#{index.underscore.tr('/', '_')}" end =begin +return true if the elastic search version is lower equal 5.6 + + result = SearchIndexBackend.lower_equal_es56? + +returns + + result = true + +=end + + def self.lower_equal_es56? + Setting.get('es_multi_index') == false + end + +=begin + generate url for index or document access (only for internal use) # url to access single document in index (in case with_pipeline or not) - url = SearchIndexBackend.build_url('User', 123, with_pipeline) + url = SearchIndexBackend.build_url(type: 'User', object_id: 123, with_pipeline: true) # url to access whole index - url = SearchIndexBackend.build_url('User') + url = SearchIndexBackend.build_url(type: 'User') # url to access document definition in index (only es6 and higher) - url = SearchIndexBackend.build_url('User', nil, false, true) + url = SearchIndexBackend.build_url(type: 'User', with_pipeline: false, with_document_type: true) # base url url = SearchIndexBackend.build_url =end - def self.build_url(type = nil, o_id = nil, with_pipeline = true, with_document_type = true) + # rubocop:disable Metrics/ParameterLists + def self.build_url(type: nil, action: nil, object_id: nil, with_pipeline: true, with_document_type: true, url_params: {}) + # rubocop:enable Metrics/ParameterLists return if !SearchIndexBackend.enabled? - # for elasticsearch 5.6 and lower - index = "#{Setting.get('es_index')}_#{Rails.env}" - if Setting.get('es_multi_index') == false - url = Setting.get('es_url') - url = if type - if with_pipeline == true - url_pipline = Setting.get('es_pipeline') - if url_pipline.present? - url_pipline = "?pipeline=#{url_pipline}" - end - end - if o_id - "#{url}/#{index}/#{type}/#{o_id}#{url_pipline}" - else - "#{url}/#{index}/#{type}#{url_pipline}" - end - else - "#{url}/#{index}" - end - return url - end + # set index + index = build_index_name(type) - # for elasticsearch 6.x and higher - url = Setting.get('es_url') - if with_pipeline == true + # add pipeline if needed + if index && with_pipeline == true url_pipline = Setting.get('es_pipeline') if url_pipline.present? - url_pipline = "?pipeline=#{url_pipline}" + url_params['pipeline'] = url_pipline end end - if type - index = build_index_name(type) - # access (e. g. creating or dropping) whole index - if with_document_type == false - return "#{url}/#{index}" - end - - # access single document in index (e. g. drop or add document) - if o_id - return "#{url}/#{index}/_doc/#{o_id}#{url_pipline}" - end - - # access document type (e. g. creating or dropping document mapping) - return "#{url}/#{index}/_doc#{url_pipline}" - end - "#{url}/" - end - -=begin - -generate url searchaccess (only for internal use) - - # url search access with single index - url = SearchIndexBackend.build_search_url('User') - - # url to access all over es - url = SearchIndexBackend.build_search_url - -=end - - def self.build_search_url(index = nil) - - # for elasticsearch 5.6 and lower - if Setting.get('es_multi_index') == false - if index - return "/#{index}/_search" - end - - return '/_search' + # prepare url params + params_string = '' + if url_params.present? + params_string = '?' + url_params.map { |key, value| "#{key}=#{value}" }.join('&') end - # for elasticsearch 6.x and higher - "#{build_index_name(index)}/_doc/_search" + url = Setting.get('es_url') + return "#{url}#{params_string}" if index.blank? + + # add type information + url = "#{url}/#{index}" + + # add document type + if with_document_type && !lower_equal_es56? + url = "#{url}/_doc" + end + + # add action + if action + url = "#{url}/#{action}" + end + + # add object id + if object_id.present? + url = "#{url}/#{object_id}" + end + + "#{url}#{params_string}" end def self.humanized_error(verb:, url:, payload: nil, response:) diff --git a/lib/tasks/search_index_es.rake b/lib/tasks/search_index_es.rake index 860d15206..2503ffe63 100644 --- a/lib/tasks/search_index_es.rake +++ b/lib/tasks/search_index_es.rake @@ -2,7 +2,7 @@ $LOAD_PATH << './lib' require 'rubygems' namespace :searchindex do - task :drop, [:opts] => :environment do |_t, _args| + task :drop, [:opts] => %i[environment searchindex:version_supported] do |_t, _args| print 'drop indexes...' # drop indexes @@ -23,7 +23,7 @@ namespace :searchindex do Rake::Task['searchindex:drop_pipeline'].execute end - task :create, [:opts] => :environment do |_t, _args| + task :create, [:opts] => %i[environment searchindex:version_supported] do |_t, _args| print 'create indexes...' if es_multi_index? @@ -67,7 +67,7 @@ namespace :searchindex do Rake::Task['searchindex:create_pipeline'].execute end - task :create_pipeline, [:opts] => :environment do |_t, _args| + task :create_pipeline, [:opts] => %i[environment searchindex:version_supported] do |_t, _args| if !es_pipeline? Setting.set('es_pipeline', '') next @@ -124,7 +124,7 @@ namespace :searchindex do puts 'done' end - task :drop_pipeline, [:opts] => :environment do |_t, _args| + task :drop_pipeline, [:opts] => %i[environment searchindex:version_supported] do |_t, _args| next if !es_pipeline? # update processors @@ -142,7 +142,7 @@ namespace :searchindex do puts 'done' end - task :reload, [:opts] => :environment do |_t, _args| + task :reload, [:opts] => %i[environment searchindex:version_supported] do |_t, _args| puts 'reload data...' Models.indexable.each do |model_class| @@ -156,17 +156,23 @@ namespace :searchindex do end - task :refresh, [:opts] => :environment do |_t, _args| + task :refresh, [:opts] => %i[environment searchindex:version_supported] do |_t, _args| print 'refresh all indexes...' SearchIndexBackend.refresh end - task :rebuild, [:opts] => :environment do |_t, _args| + task :rebuild, [:opts] => %i[environment searchindex:version_supported] do |_t, _args| Rake::Task['searchindex:drop'].execute Rake::Task['searchindex:create'].execute Rake::Task['searchindex:reload'].execute end + + task :version_supported, [:opts] => :environment do |_t, _args| + next if es_version_supported? + + abort "Your elastic search version is not supported! Please update your version to a greater equal than 5.6.0 (Your current version: #{es_version})." + end end =begin @@ -300,6 +306,16 @@ def es_version end end +def es_version_supported? + version_split = es_version.split('.') + version = "#{version_split[0]}#{format('%03d', version_split[1])}#{format('%03d', version_split[2])}".to_i + + # only versions greater/equal than 5.6.0 are supported + return if version < 5_006_000 + + true +end + # no es_pipeline for elasticsearch 5.5 and lower def es_pipeline? number = es_version diff --git a/spec/models/organization_spec.rb b/spec/models/organization_spec.rb index f91884d78..253b1c7fc 100644 --- a/spec/models/organization_spec.rb +++ b/spec/models/organization_spec.rb @@ -31,39 +31,12 @@ RSpec.describe Organization, type: :model do let!(:member) { create(:customer_user, organization: organization) } let!(:member_ticket) { create(:ticket, customer: member) } - context 'when basic attributes are updated' do - it 'touches its members and their tickets' do - expect { organization.update(name: 'foo') } - .to change { member.reload.updated_at } - .and change { member_ticket.reload.updated_at } - end - end - context 'when member associations are added' do let(:user) { create(:customer_user) } it 'is touched, and touches its other members (but not their tickets)' do expect { organization.members.push(user) } .to change { organization.reload.updated_at } - .and change { member.reload.updated_at } - .and not_change { member_ticket.reload.updated_at } - end - end - - context 'with 100+ members' do - let!(:members) { create_list(:user, 101, organization: organization) } - let!(:member_ticket) { create(:ticket, customer: members.first) } - - # This _should_ be split into two separate examples, - # but setup is slow and expensive. - it 'does not perform any association updates' do - expect { organization.update(name: 'foo') } - .to not_change { members.map(&:reload).map(&:updated_at) } - .and not_change { member_ticket.reload.updated_at } - - expect { organization.members.push(member) } - .to not_change { organization.reload.updated_at } - .and not_change { members.map(&:reload).map(&:updated_at) } end end end diff --git a/spec/models/ticket_spec.rb b/spec/models/ticket_spec.rb index c37a1a148..964c507bc 100644 --- a/spec/models/ticket_spec.rb +++ b/spec/models/ticket_spec.rb @@ -852,20 +852,6 @@ RSpec.describe Ticket, type: :model do .and change { other_organization.reload.updated_at } end end - - context 'when organization has 100+ members' do - let!(:other_members) { create_list(:user, 100, organization: organization) } - - context 'and customer association is changed' do - it 'touches both old and new customer, and their organizations' do - expect { ticket.update(customer: other_customer) } - .to change { customer.reload.updated_at } - .and change { organization.reload.updated_at } - .and change { other_customer.reload.updated_at } - .and change { other_organization.reload.updated_at } - end - end - end end describe 'Association & attachment management:' do diff --git a/spec/models/user/can_lookup_search_index_attributes_examples.rb b/spec/models/user/can_lookup_search_index_attributes_examples.rb new file mode 100644 index 000000000..0e5b4a26d --- /dev/null +++ b/spec/models/user/can_lookup_search_index_attributes_examples.rb @@ -0,0 +1,38 @@ +RSpec.shared_examples 'CanLookupSearchIndexAttributes' do + describe '.search_index_value_by_attribute' do + it 'returns hash of data' do + organization = create(:organization, name: 'Tomato42', note: 'special recipe') + user = create(:agent_user, organization: organization) + + value = user.search_index_value_by_attribute('organization_id') + expect_value = { 'name' => 'Tomato42', 'note' => 'special recipe' } + expect(value).to be_a_kind_of(Hash) + expect(value).to eq(expect_value) + end + end + + describe '.search_index_value' do + it 'returns correct value' do + organization = create(:organization, name: 'Tomato42', note: 'special recipe') + + value = organization.search_index_value + expect_value = { 'name' => 'Tomato42', 'note' => 'special recipe' } + expect(value).to be_a_kind_of(Hash) + expect(value).to eq(expect_value) + end + end + + describe '.search_index_attribute_ref_name' do + it 'returns correct value' do + attribute_ref_name = User.search_index_attribute_ref_name('organization_id') + expect(attribute_ref_name).to eq('organization') + end + end + + describe '.search_index_attribute_ignored?' do + it 'returns correct value' do + ignored = User.search_index_attribute_ignored?('password') + expect(ignored).to be true + end + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 8c515559f..1f9b36bb7 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -7,6 +7,7 @@ require 'models/concerns/has_groups_permissions_examples' require 'models/concerns/has_xss_sanitized_note_examples' require 'models/concerns/can_be_imported_examples' require 'models/concerns/has_object_manager_attributes_validation_examples' +require 'models/user/can_lookup_search_index_attributes_examples' RSpec.describe User, type: :model do subject(:user) { create(:user) } @@ -23,6 +24,7 @@ RSpec.describe User, type: :model do it_behaves_like 'HasGroups and Permissions', group_access_no_permission_factory: :user it_behaves_like 'CanBeImported' it_behaves_like 'HasObjectManagerAttributesValidation' + it_behaves_like 'CanLookupSearchIndexAttributes' describe 'Class methods:' do describe '.authenticate' do @@ -1142,28 +1144,16 @@ RSpec.describe User, type: :model do end describe 'Touching associations on update:' do - subject(:user) { create(:customer_user, organization: organization) } + subject!(:user) { create(:customer_user) } - let(:organization) { create(:organization) } - let(:other_customer) { create(:customer_user) } + let!(:organization) { create(:organization) } - context 'when basic attributes are updated' do + context 'when a customer gets a organization ' do it 'touches its organization' do - expect { user.update(firstname: 'foo') } + expect { user.update(organization: organization) } .to change { organization.reload.updated_at } end end - - context 'when organization has 100+ other members' do - let!(:other_members) { create_list(:user, 100, organization: organization) } - - context 'and basic attributes are updated' do - it 'does not touch its organization' do - expect { user.update(firstname: 'foo') } - .to not_change { organization.reload.updated_at } - end - end - end end describe 'Cti::CallerId syncing:' do diff --git a/spec/requests/organization_spec.rb b/spec/requests/organization_spec.rb index 5c5938365..973b66c18 100644 --- a/spec/requests/organization_spec.rb +++ b/spec/requests/organization_spec.rb @@ -106,24 +106,27 @@ RSpec.describe 'Organization', type: :request, searchindex: true do get "/api/v1/organizations/search?query=#{CGI.escape('Zammad')}", params: {}, as: :json expect(response).to have_http_status(:ok) expect(json_response).to be_a_kind_of(Array) - expect(json_response[0]['name']).to eq('Zammad Foundation') - expect(json_response[0]['member_ids']).to be_truthy - expect(json_response[0]['members']).to be_falsey + organization = json_response.detect { |object| object['name'] == 'Zammad Foundation' } + expect(organization['name']).to eq('Zammad Foundation') + expect(organization['member_ids']).to be_truthy + expect(organization['members']).to be_falsey get "/api/v1/organizations/search?query=#{CGI.escape('Zammad')}&expand=true", params: {}, as: :json expect(response).to have_http_status(:ok) expect(json_response).to be_a_kind_of(Array) - expect(json_response[0]['name']).to eq('Zammad Foundation') - expect(json_response[0]['member_ids']).to be_truthy - expect(json_response[0]['members']).to be_truthy + organization = json_response.detect { |object| object['name'] == 'Zammad Foundation' } + expect(organization['name']).to eq('Zammad Foundation') + expect(organization['member_ids']).to be_truthy + expect(organization['members']).to be_truthy get "/api/v1/organizations/search?query=#{CGI.escape('Zammad')}&label=true", params: {}, as: :json expect(response).to have_http_status(:ok) expect(json_response).to be_a_kind_of(Array) - expect(json_response[0]['label']).to eq('Zammad Foundation') - expect(json_response[0]['value']).to eq('Zammad Foundation') - expect(json_response[0]['member_ids']).to be_falsey - expect(json_response[0]['members']).to be_falsey + organization = json_response.detect { |object| object['label'] == 'Zammad Foundation' } + expect(organization['label']).to eq('Zammad Foundation') + expect(organization['value']).to eq('Zammad Foundation') + expect(organization['member_ids']).to be_falsey + expect(organization['members']).to be_falsey end it 'does index with customer1' do diff --git a/spec/requests/search_spec.rb b/spec/requests/search_spec.rb index 958888184..fb64c5f17 100644 --- a/spec/requests/search_spec.rb +++ b/spec/requests/search_spec.rb @@ -27,6 +27,12 @@ RSpec.describe 'Search', type: :request, searchindex: true do let!(:organization5) do create(:organization, name: 'ABC_D Org') end + let!(:organization_nested) do + create(:organization, name: 'Tomato42 Ltd.', note: 'Tomato42 Ltd.') + end + let!(:customer_user_nested) do + create(:customer_user, organization: organization_nested) + end let!(:customer_user2) do create(:customer_user, organization: organization1) end @@ -42,6 +48,9 @@ RSpec.describe 'Search', type: :request, searchindex: true do let!(:ticket3) do create(:ticket, title: 'test 1234-2', customer: customer_user3, group: group) end + let!(:ticket_nested) do + create(:ticket, title: 'vegetable request', customer: customer_user_nested, group: group) + end let!(:article1) do create(:ticket_article, ticket_id: ticket1.id) end @@ -51,6 +60,20 @@ RSpec.describe 'Search', type: :request, searchindex: true do let!(:article3) do create(:ticket_article, ticket_id: ticket3.id) end + let!(:article_nested) do + article = create(:ticket_article, ticket_id: ticket_nested.id) + + Store.add( + object: 'Ticket::Article', + o_id: article.id, + data: File.binread(Rails.root.join('test', 'data', 'elasticsearch', 'es-normal.txt')), + filename: 'es-normal.txt', + preferences: {}, + created_by_id: 1, + ) + + article + end before do configure_elasticsearch do @@ -327,5 +350,100 @@ RSpec.describe 'Search', type: :request, searchindex: true do target_id = json_response['result'][0]['id'] expect(json_response['assets']['Organization'][target_id.to_s]['name']).to eq('ABC_D Org') end + + it 'does find the user of the nested organization and also even if the organization name changes' do + params = { + query: 'Tomato42', + limit: 10, + } + + # because of the initial relation between user and organization + # both user and organization will be found as result + authenticated_as(agent_user) + post '/api/v1/search/User', params: params, as: :json + expect(response).to have_http_status(:ok) + expect(json_response).to be_a_kind_of(Hash) + expect(json_response).to be_truthy + expect(json_response['assets']['Organization'][organization_nested.id.to_s]).to be_truthy + expect(json_response['assets']['User'][customer_user_nested.id.to_s]).to be_truthy + + organization_nested.update(name: 'Cucumber43 Ltd.') + Scheduler.worker(true) + SearchIndexBackend.refresh + + params = { + query: 'Cucumber43', + limit: 10, + } + + # even after a change of the organization name we should find + # the customer user because of the nested organization data + post '/api/v1/search/User', params: params, as: :json + expect(response).to have_http_status(:ok) + expect(json_response).to be_a_kind_of(Hash) + expect(json_response).to be_truthy + expect(json_response['assets']['Organization'][organization_nested.id.to_s]).to be_truthy + expect(json_response['assets']['User'][customer_user_nested.id.to_s]).to be_truthy + end + + it 'does find the ticket by organization name even if the organization name changes' do + params = { + query: 'Tomato42', + limit: 10, + } + + authenticated_as(agent_user) + post '/api/v1/search/Ticket', params: params, as: :json + expect(response).to have_http_status(:ok) + expect(json_response).to be_a_kind_of(Hash) + expect(json_response).to be_truthy + expect(json_response['assets']['Organization'][organization_nested.id.to_s]).to be_truthy + expect(json_response['assets']['Ticket'][ticket_nested.id.to_s]).to be_truthy + + organization_nested.update(name: 'Cucumber43 Ltd.') + Scheduler.worker(true) + SearchIndexBackend.refresh + + params = { + query: 'Cucumber43', + limit: 10, + } + + post '/api/v1/search/Ticket', params: params, as: :json + expect(response).to have_http_status(:ok) + expect(json_response).to be_a_kind_of(Hash) + expect(json_response).to be_truthy + expect(json_response['assets']['Organization'][organization_nested.id.to_s]).to be_truthy + expect(json_response['assets']['Ticket'][ticket_nested.id.to_s]).to be_truthy + end + + it 'does find the ticket by attachment even after ticket reindex' do + params = { + query: 'text66', + limit: 10, + } + + authenticated_as(agent_user) + post '/api/v1/search/Ticket', params: params, as: :json + expect(response).to have_http_status(:ok) + expect(json_response).to be_a_kind_of(Hash) + expect(json_response).to be_truthy + expect(json_response['assets']['Ticket'][ticket_nested.id.to_s]).to be_truthy + + organization_nested.update(name: 'Cucumber43 Ltd.') + Scheduler.worker(true) + SearchIndexBackend.refresh + + params = { + query: 'text66', + limit: 10, + } + + post '/api/v1/search/Ticket', params: params, as: :json + expect(response).to have_http_status(:ok) + expect(json_response).to be_a_kind_of(Hash) + expect(json_response).to be_truthy + expect(json_response['assets']['Ticket'][ticket_nested.id.to_s]).to be_truthy + end end end diff --git a/spec/requests/user/organization_spec.rb b/spec/requests/user/organization_spec.rb index 27b549e6b..1fef2117e 100644 --- a/spec/requests/user/organization_spec.rb +++ b/spec/requests/user/organization_spec.rb @@ -89,24 +89,27 @@ RSpec.describe 'User Organization', type: :request, searchindex: true do get "/api/v1/organizations/search?query=#{CGI.escape('Zammad')}", params: {}, as: :json expect(response).to have_http_status(:ok) expect(json_response.class).to eq(Array) - expect(json_response[0]['name']).to eq('Zammad Foundation') - expect(json_response[0]['member_ids']).to be_truthy - expect(json_response[0]['members']).to be_falsey + organization = json_response.detect { |object| object['name'] == 'Zammad Foundation' } + expect(organization['name']).to eq('Zammad Foundation') + expect(organization['member_ids']).to be_truthy + expect(organization['members']).to be_falsey get "/api/v1/organizations/search?query=#{CGI.escape('Zammad')}&expand=true", params: {}, as: :json expect(response).to have_http_status(:ok) expect(json_response.class).to eq(Array) - expect(json_response[0]['name']).to eq('Zammad Foundation') - expect(json_response[0]['member_ids']).to be_truthy - expect(json_response[0]['members']).to be_truthy + organization = json_response.detect { |object| object['name'] == 'Zammad Foundation' } + expect(organization['name']).to eq('Zammad Foundation') + expect(organization['member_ids']).to be_truthy + expect(organization['members']).to be_truthy get "/api/v1/organizations/search?query=#{CGI.escape('Zammad')}&label=true", params: {}, as: :json expect(response).to have_http_status(:ok) expect(json_response.class).to eq(Array) - expect(json_response[0]['label']).to eq('Zammad Foundation') - expect(json_response[0]['value']).to eq('Zammad Foundation') - expect(json_response[0]['member_ids']).to be_falsey - expect(json_response[0]['members']).to be_falsey + organization = json_response.detect { |object| object['label'] == 'Zammad Foundation' } + expect(organization['label']).to eq('Zammad Foundation') + expect(organization['value']).to eq('Zammad Foundation') + expect(organization['member_ids']).to be_falsey + expect(organization['members']).to be_falsey end it 'does organization index with customer1' do diff --git a/test/integration/elasticsearch_test.rb b/test/integration/elasticsearch_test.rb index acfac7d0e..bbbdd0be0 100644 --- a/test/integration/elasticsearch_test.rb +++ b/test/integration/elasticsearch_test.rb @@ -74,6 +74,7 @@ class ElasticsearchTest < ActiveSupport::TestCase # execute background jobs to index created/changed objects Scheduler.worker(true) + SearchIndexBackend.refresh end @@ -103,7 +104,7 @@ class ElasticsearchTest < ActiveSupport::TestCase assert_equal('es-customer1@example.com', attributes['email']) assert(attributes['preferences']) assert_not(attributes['password']) - assert_equal('Customer Organization Update', attributes['organization']) + assert_equal({ 'name' => 'Customer Organization Update', 'note' => 'some note' }, attributes['organization']) # organization attributes = @organization1.search_index_data @@ -174,6 +175,7 @@ class ElasticsearchTest < ActiveSupport::TestCase # execute background jobs Scheduler.worker(true) + SearchIndexBackend.refresh ticket1 = Ticket.create!( title: "some title\n äöüß", @@ -295,7 +297,7 @@ class ElasticsearchTest < ActiveSupport::TestCase # execute background jobs Scheduler.worker(true) - sleep 2 # for ES to come ready/indexed + SearchIndexBackend.refresh # search as @agent @@ -433,7 +435,7 @@ class ElasticsearchTest < ActiveSupport::TestCase # execute background jobs Scheduler.worker(true) - sleep 2 # for ES to come ready/indexed + SearchIndexBackend.refresh # search for tags result = Ticket.search( diff --git a/test/unit/user_assets_test.rb b/test/unit/user_assets_test.rb index 203a02864..b87f7876e 100644 --- a/test/unit/user_assets_test.rb +++ b/test/unit/user_assets_test.rb @@ -94,60 +94,10 @@ class UserAssetsTest < ActiveSupport::TestCase attributes.delete('authorization_ids') assert(diff(attributes, assets[:User][user3.id]), 'check assets') - # touch org, check if user1 has changed - travel 2.seconds - org2 = Organization.find(org1.id) - org2.note = "some note...#{rand(9_999_999_999_999)}" - org2.save! - - attributes = org2.attributes_with_association_ids - attributes.delete('user_ids') - assert_not(diff(attributes, assets[:Organization][org2.id]), 'check assets') - - user1_new = User.find(user1.id) - attributes = user1_new.attributes_with_association_ids - attributes['accounts'] = {} - attributes.delete('password') - attributes.delete('token_ids') - attributes.delete('authorization_ids') - assert_not(diff(attributes, assets[:User][user1_new.id]), 'check assets') - - # check new assets lookup - assets = user3.assets({}) - attributes = org2.attributes_with_association_ids - attributes.delete('user_ids') - assert(diff(attributes, assets[:Organization][org1.id]), 'check assets') - - user1 = User.find(user1.id) - attributes = user1.attributes_with_association_ids - attributes['accounts'] = {} - attributes.delete('password') - attributes.delete('token_ids') - attributes.delete('authorization_ids') - assert(diff(attributes, assets[:User][user1.id]), 'check assets') - - user2 = User.find(user2.id) - attributes = user2.attributes_with_association_ids - attributes['accounts'] = {} - attributes.delete('password') - attributes.delete('token_ids') - attributes.delete('authorization_ids') - assert(diff(attributes, assets[:User][user2.id]), 'check assets') - - user3 = User.find(user3.id) - attributes = user3.attributes_with_association_ids - attributes['accounts'] = {} - attributes.delete('password') - attributes.delete('token_ids') - attributes.delete('authorization_ids') - assert(diff(attributes, assets[:User][user3.id]), 'check assets') - travel_back - user3.destroy! user2.destroy! user1.destroy! org1.destroy! - assert_not(Organization.find_by(id: org2.id)) end def diff(object1, object2)