For Elasticsearch, sort by active flag first before everything else

.gitlab-ci.yml

@@ -240,6 +240,7 @@ test:integration:es_mysql:
     - export ES_URL="http://localhost:9200"
     - rake db:create
     - rake db:migrate
+    - ruby -I test/ test/integration/elasticsearch_active_test.rb
     - ruby -I test/ test/integration/elasticsearch_test.rb
     - ruby -I test/ test/controllers/search_controller_test.rb
     - ruby -I test/ test/integration/report_test.rb
@@ -259,6 +260,7 @@ test:integration:es_postgresql:
     - export ES_URL="http://localhost:9200"
     - rake db:create
     - rake db:migrate
+    - ruby -I test/ test/integration/elasticsearch_active_test.rb
     - ruby -I test/ test/integration/elasticsearch_test.rb
     - ruby -I test/ test/controllers/search_controller_test.rb
     - ruby -I test/ test/integration/report_test.rb

lib/search_index_backend.rb

@@ -413,6 +413,15 @@ return search result
       )
     end
 
+    # add sorting by active if active is not part of the query
+    if result.flat_map(&:keys).exclude?(:active)
+      result.unshift(
+        active: {
+          order: 'desc',
+        },
+      )
+    end
+
     result.push('_score')
 
     result

test/integration/elasticsearch_active_test.rb

@@ -0,0 +1,125 @@
+require 'test_helper'
+
+class ElasticsearchActiveTest < ActiveSupport::TestCase
+  include SearchindexHelper
+
+  setup do
+
+    configure_elasticsearch(required: true)
+
+    rebuild_searchindex
+
+    roles  = Role.where(name: 'Agent')
+    groups = Group.where(name: 'Users')
+
+    @agent = User.create!(
+      login: 'es-agent@example.com',
+      firstname: 'E',
+      lastname: 'S',
+      email: 'es-agent@example.com',
+      password: 'agentpw',
+      active: true,
+      roles: roles,
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+
+    roles = Role.where(name: 'Customer')
+
+    (1..6).each do |i|
+      name = i.even? ? "Active-#{i}" : "Inactive-#{i}"
+      User.create!(
+        login: "#{name}-customer#{i}@example.com",
+        firstname: 'ActiveTest',
+        lastname: name,
+        active: i.even?,
+        roles: roles,
+        updated_by_id: 1,
+        created_by_id: 1,
+      )
+      Organization.create!(
+        name: "TestOrg-#{name}",
+        active: i.even?,
+        updated_by_id: 1,
+        created_by_id: 1,
+      )
+      travel 10.seconds
+    end
+
+    # execute background jobs to index created/changed objects
+    Scheduler.worker(true)
+    sleep 2 # for ES to come ready/indexed
+  end
+
+  test 'active users appear before inactive users in search results' do
+    result = User.search(
+      current_user: @agent,
+      query: 'ActiveTest',
+      limit: 15,
+    )
+    assert(result.present?, 'result should not be empty')
+
+    names = result.map(&:lastname)
+    correct_names = %w[Active-6 Active-4 Active-2 Inactive-5 Inactive-3 Inactive-1]
+    assert_equal(correct_names, names)
+  end
+
+  test 'active organizations appear before inactive organizations in search results' do
+    result = Organization.search(
+      current_user: @agent,
+      query: 'TestOrg',
+      limit: 15,
+    )
+    assert(result.present?, 'result should not be empty')
+
+    names = result.map(&:name)
+    correct_names = %w[TestOrg-Active-2
+                       TestOrg-Active-4
+                       TestOrg-Active-6
+                       TestOrg-Inactive-1
+                       TestOrg-Inactive-3
+                       TestOrg-Inactive-5]
+    assert_equal(correct_names, names)
+  end
+
+  test 'ordering of tickets are not affected by the lack of active flags' do
+    ticket_setup
+
+    result = Ticket.search(
+      current_user: User.find(1),
+      query: 'ticket',
+      limit: 15,
+    )
+    assert(result.present?, 'result should not be empty')
+
+    names = result.map(&:title)
+    correct_names = %w[Ticket-6 Ticket-5 Ticket-4 Ticket-3 Ticket-2 Ticket-1]
+    assert_equal(correct_names, names)
+  end
+
+  def ticket_setup
+    result = Ticket.search(
+      current_user: User.find(1),
+      query: 'ticket',
+      limit: 15,
+    )
+    return if result.present?
+
+    (1..6).each do |i|
+      Ticket.create!(
+        title: "Ticket-#{i}",
+        group: Group.lookup(name: 'Users'),
+        customer_id: 1,
+        state: Ticket::State.lookup(name: 'new'),
+        priority: Ticket::Priority.lookup(name: '2 normal'),
+        updated_by_id: 1,
+        created_by_id: 1,
+      )
+      travel 10.seconds
+    end
+
+    # execute background jobs to index created/changed objects
+    Scheduler.worker(true)
+    sleep 2 # for ES to come ready/indexed
+  end
+end

test/integration/elasticsearch_test.rb

@@ -1,4 +1,4 @@
-require 'integration_test_helper'
+require 'test_helper'
 
 class ElasticsearchTest < ActiveSupport::TestCase
   include SearchindexHelper
@@ -11,7 +11,7 @@ class ElasticsearchTest < ActiveSupport::TestCase
 
     groups = Group.where(name: 'Users')
     roles  = Role.where(name: 'Agent')
-    @agent = User.create_or_update(
+    @agent = User.create!(
       login: 'es-agent@example.com',
       firstname: 'E',
       lastname: 'S',
@@ -36,7 +36,7 @@ class ElasticsearchTest < ActiveSupport::TestCase
       updated_by_id: 1,
       created_by_id: 1,
     )
-    @customer1 = User.create_or_update(
+    @customer1 = User.create!(
       login: 'es-customer1@example.com',
       firstname: 'ES',
       lastname: 'Customer1',
@@ -48,8 +48,7 @@ class ElasticsearchTest < ActiveSupport::TestCase
       updated_by_id: 1,
       created_by_id: 1,
     )
-    sleep 1
+    @customer2 = User.create!(
-    @customer2 = User.create_or_update(
       login: 'es-customer2@example.com',
       firstname: 'ES',
       lastname: 'Customer2',
@@ -61,8 +60,7 @@ class ElasticsearchTest < ActiveSupport::TestCase
       updated_by_id: 1,
       created_by_id: 1,
     )
-    sleep 1
+    @customer3 = User.create!(
-    @customer3 = User.create_or_update(
       login: 'es-customer3@example.com',
       firstname: 'ES',
       lastname: 'Customer3',
@@ -73,6 +71,10 @@ class ElasticsearchTest < ActiveSupport::TestCase
       updated_by_id: 1,
       created_by_id: 1,
     )
+
+    # execute background jobs to index created/changed objects
+    Scheduler.worker(true)
+
   end
 
   # check search attributes
@@ -240,7 +242,7 @@ class ElasticsearchTest < ActiveSupport::TestCase
       created_by_id: 1,
     )
     ticket1.tag_add('someTagA', 1)
-    sleep 1
+    travel 1.minute
 
     ticket2 = Ticket.create!(
       title: 'something else',
@@ -266,7 +268,7 @@ class ElasticsearchTest < ActiveSupport::TestCase
       created_by_id: 1,
     )
     ticket2.tag_add('someTagB', 1)
-    sleep 1
+    travel 1.minute
 
     ticket3 = Ticket.create!(
       title: 'something else',
@@ -293,7 +295,7 @@ class ElasticsearchTest < ActiveSupport::TestCase
 
     # execute background jobs
     Scheduler.worker(true)
-    sleep 4
+    sleep 2 # for ES to come ready/indexed
 
     # search as @agent
 
@@ -431,8 +433,7 @@ class ElasticsearchTest < ActiveSupport::TestCase
 
     # execute background jobs
     Scheduler.worker(true)
-
+    sleep 2 # for ES to come ready/indexed
-    sleep 4
 
     # search for tags
     result = Ticket.search(

test/support/searchindex_helper.rb

@@ -43,15 +43,7 @@ module SearchindexHelper
   def rebuild_searchindex
     Rake::Task.clear
     Zammad::Application.load_tasks
-
+    Rake::Task['searchindex:rebuild'].execute
-    if ENV['ES_INDEX_RAND'].blank?
-      Rake::Task['searchindex:rebuild'].execute
-    else
-      # if we have a random index we don't need
-      # to drop the index in the first place
-      Rake::Task['searchindex:create'].execute
-      Rake::Task['searchindex:reload'].execute
-    end
   end
 
 end