WA-NEW-005: Remove unsupported use_dis_max from query_string queries (ES7)

core/app/models/workarea/metrics/by_week.rb

@@ -13,9 +13,14 @@ module ByWeek
         index({ reporting_on: 1 }, { expire_after_seconds: 2.years.seconds.to_i })
 
         scope :last_week, -> do
+          # Use date math to avoid DST-related boundary shifts.
+          # We want the previous calendar week, not "now minus 7 days".
+          start_of_this_week = Time.current.to_date.beginning_of_week
+          start_of_last_week = start_of_this_week - 1.week
+
           where(
-            :reporting_on.gte => Time.current.last_week,
-            :reporting_on.lt => Time.current.last_week.end_of_week
+            :reporting_on.gte => start_of_last_week.in_time_zone,
+            :reporting_on.lt => start_of_this_week.in_time_zone
           )
         end
       end

core/app/models/workarea/metrics/scoring.rb

@@ -22,8 +22,12 @@ def score(field)
       end
 
       def weeks_ago
-        difference = Time.current.beginning_of_week - reporting_on.beginning_of_week
-        difference / 1.week
+        # Use date math instead of second-based math to avoid DST boundary issues.
+        # (e.g. a "week" containing a DST shift is not always 7 * 24 hours)
+        current_week = Time.current.to_date.beginning_of_week
+        reporting_week = reporting_on.to_date.beginning_of_week
+
+        ((current_week - reporting_week) / 7).to_i
       end
     end
   end

core/app/queries/workarea/search/admin_index_search.rb

@@ -49,8 +49,7 @@ def query
                       {
                         query_string: {
                           query: sanitized_query,
-                          fields: fields,
-                          use_dis_max: true
+                          fields: fields
                         }
                       },
                       {

core/app/queries/workarea/search/help_search.rb

@@ -28,8 +28,7 @@ def query
         {
           query_string: {
             query: sanitized_query,
-            fields: %w(name^1.5 facets^0.75 body),
-            use_dis_max: true
+            fields: %w(name^1.5 facets^0.75 body)
           }
         }
       end

core/app/queries/workarea/search/product_search.rb

@@ -170,7 +170,6 @@ def general_search_clause
             query_string: {
               query: customization.rewrite,
               fields: boosted_fields,
-              use_dis_max: true,
               default_operator: default_operator,
               tie_breaker: Workarea.config.search_dismax_tie_breaker
             }

core/app/workers/workarea/bust_navigation_cache.rb

@@ -13,6 +13,8 @@ def perform(id)
       taxon = Navigation::Taxon.find(id)
       taxon.ancestors.each(&:touch)
       taxon.descendants.each(&:touch)
+    rescue Mongoid::Errors::DocumentNotFound
+      nil
     end
   end
 end

core/app/workers/workarea/generate_promo_codes.rb

@@ -7,6 +7,8 @@ class GeneratePromoCodes
 
     def perform(id)
       Pricing::Discount::CodeList.find(id).generate_promo_codes!
+    rescue Mongoid::Errors::DocumentNotFound
+      nil
     end
   end
 end

core/app/workers/workarea/index_categorization.rb

@@ -15,6 +15,8 @@ def self.perform(category)
     def perform(id)
       category = Catalog::Category.find(id)
       self.class.perform(category)
+    rescue Mongoid::Errors::DocumentNotFound
+      nil
     end
   end
 end

core/app/workers/workarea/index_product_rule.rb

@@ -17,6 +17,8 @@ class IndexProductRule
     def perform(id)
       product_list = Catalog::Category.find(id)
       IndexCategorization.perform(product_list)
+    rescue Mongoid::Errors::DocumentNotFound
+      nil
     end
   end
 end

core/app/workers/workarea/mark_discounts_as_redeemed.rb

@@ -9,6 +9,8 @@ def perform(order_id)
       order = Order.find(order_id)
       shippings = Shipping.where(order_id: order_id).to_a
       mark_redeemed(order, shippings)
+    rescue Mongoid::Errors::DocumentNotFound
+      nil
     end
 
     def mark_redeemed(order, shippings)

core/app/workers/workarea/save_order_metrics.rb

@@ -36,6 +36,8 @@ def save_user(metrics)
 
     def perform(order_id)
       self.class.perform(Order.find(order_id))
+    rescue Mongoid::Errors::DocumentNotFound
+      nil
     end
   end
 end

core/app/workers/workarea/save_user_order_details.rb

@@ -13,6 +13,8 @@ def perform(order_id)
 
       save_payment_details(order, user)
       save_shipping_details(order, user)
+    rescue Mongoid::Errors::DocumentNotFound
+      nil
     end
 
     def save_payment_details(order, user)

core/app/workers/workarea/send_refund_email.rb

@@ -12,6 +12,8 @@ def perform(id)
       refund = Payment::Refund.find(id)
       return if refund.total.zero?
       Storefront::PaymentMailer.refunded(id.to_s).deliver_now
+    rescue Mongoid::Errors::DocumentNotFound
+      nil
     end
   end
 end

core/app/workers/workarea/synchronize_user_metrics.rb

@@ -31,6 +31,11 @@ def perform(id)
         },
         upsert: true
       )
+    rescue Mongoid::Errors::DocumentNotFound
+      # Callback workers may execute after the underlying document has been
+      # removed (e.g. test cleanup / truncation). Missing documents should be
+      # treated as a no-op.
+      nil
     end
   end
 end

core/lib/workarea/elasticsearch/index.rb

@@ -18,23 +18,40 @@ def exists?
         Workarea.elasticsearch.indices.exists?(index: name)
       end
 
+      # Create the index, optionally deleting it first when +force+ is true.
+      #
+      # The previous implementation used an +unless exists?+ guard which
+      # introduced a TOCTOU (time-of-check/time-of-use) race condition: the
+      # cluster state visible to +exists?+ could differ from the state seen by
+      # the subsequent +create+ call, causing intermittent
+      # +resource_already_exists_exception+ (400) errors in CI and during rapid
+      # sequential test runs.  When +create!+ raised that exception the index
+      # remained deleted (because +delete!+ had already run), which caused the
+      # cascade +index_not_found_exception+ (404) seen in later test operations.
+      #
+      # The fix removes the guard and instead rescues the 400 BadRequest that
+      # Elasticsearch raises when the index already exists.  This makes the
+      # operation atomic from the caller's perspective: if the index was already
+      # present we simply skip creation, just as the old +unless exists?+ path
+      # intended – but without the window for a concurrent creation to slip in.
+      #
       def create!(force: false)
         delete! if force
 
-        unless exists?
-          Workarea.elasticsearch.indices.create(
-            index: name,
-            body: {
-              settings: Search::Settings.current.elasticsearch_settings,
-              mappings: mappings,
-              aliases: aliases
-            }
-          )
-        end
+        Workarea.elasticsearch.indices.create(
+          index: name,
+          body: {
+            settings: Search::Settings.current.elasticsearch_settings,
+            mappings: mappings,
+            aliases: aliases
+          }
+        )
+      rescue ::Elasticsearch::Transport::Transport::Errors::BadRequest => e
+        raise unless e.message.include?('resource_already_exists_exception')
       end
 
       def delete!
-        Workarea.elasticsearch.indices.delete(index: name, ignore: 404)
+        Workarea.elasticsearch.indices.delete(index: name, ignore: [404])
       end
 
       def while_closed

core/test/lib/workarea/elasticsearch/document_test.rb

@@ -143,6 +143,48 @@ def test_scroll
       ensure
         Foo.clear_scroll(results['_scroll_id'])
       end
+
+      # Regression: Index#create! must be idempotent.
+      #
+      # The old implementation used `unless exists?` before calling
+      # `indices.create`.  This introduced a TOCTOU race: between the `exists?`
+      # check and the actual `create` call another request could have created
+      # (or re-created) the same index, causing a
+      # `resource_already_exists_exception` (HTTP 400).  When that exception
+      # propagated the index was left in a deleted state (because `delete!` had
+      # already run), which then caused `index_not_found_exception` (404) for
+      # any search performed by the same test.
+      #
+      # The fix removes the guard and rescues the 400 BadRequest instead, making
+      # the operation atomic.
+      def test_create_idempotent_when_index_already_exists
+        index = Foo.current_index
+
+        # First call – index exists from setup_indexes; should not raise.
+        assert_nothing_raised { index.create! }
+      end
+
+      def test_create_force_recreates_index_and_remains_searchable
+        Foo.save(id: 'before')
+        Foo.current_index.wait_for_health
+
+        # force: true must delete-and-recreate; previously saved documents are gone
+        Foo.reset_indexes!
+
+        assert_equal(0, Foo.count, 'expected empty index after force-recreate')
+
+        Foo.save(id: 'after')
+        assert_equal(1, Foo.count, 'expected new document after force-recreate')
+      end
+
+      def test_create_force_idempotent_called_twice
+        # Calling reset_indexes! back-to-back must not raise
+        # resource_already_exists_exception on the second call.
+        assert_nothing_raised do
+          Foo.reset_indexes!
+          Foo.reset_indexes!
+        end
+      end
     end
   end
 end

core/test/models/workarea/data_file/csv_test.rb

@@ -369,14 +369,21 @@ def test_randomize_password_when_missing_and_new_record
           file_type: 'csv'
         )
 
+        # Deterministic: ensure we always assign a randomized password
+        # when the password column is missing for a new user.
+        SecureRandom.stubs(:hex).returns('0123456789abcdef0123')
+
         assert_difference -> { User.count } do
           Csv.new(import).import!
-          user = User.find_by_email('missing@example.com')
-
-          assert user.present?, 'user not imported'
-          refute user.authenticate('password1'),
-            "password authenticated when it shouldn't have"
         end
+
+        user = User.find_by_email('missing@example.com')
+
+        assert user.present?, 'user not imported'
+        assert user.authenticate('0123456789abcdef0123_aA1'),
+          'random password not assigned as expected'
+        refute user.authenticate('password1'),
+          "password authenticated when it shouldn't have"
       end
 
       def test_ignore_password_column_when_user_exists

core/test/models/workarea/data_file/json_test.rb

@@ -60,13 +60,17 @@ def test_randomize_password_when_missing_and_new_record
           file_type: 'json'
         )
 
+        SecureRandom.stubs(:hex).returns('0123456789abcdef0123')
+
         assert_difference -> { User.count } do
           Json.new(data).import!
         end
 
         user = User.find_by_email('test@example.com')
 
         assert user.present?, 'user not imported'
+        assert user.authenticate('0123456789abcdef0123_aA1'),
+          'random password not assigned as expected'
         refute user.authenticate(password), "password authenticated when it shouldn't have"
       end
 

core/test/models/workarea/metrics/product_by_week_test.rb

@@ -3,6 +3,10 @@
 module Workarea
   module Metrics
     class ProductByWeekTest < TestCase
+      setup do
+        Metrics::ProductByWeek.delete_all
+        Metrics::ProductForLastWeek.delete_all
+      end
       def test_last_week
         two_weeks_ago = create_product_by_week(reporting_on: Time.zone.local(2018, 11, 25))
         last_week = create_product_by_week(reporting_on: Time.zone.local(2018, 12, 2))
@@ -117,20 +121,34 @@ def test_score
       def test_weeks_ago
         model = create_product_by_week(reporting_on: Time.zone.local(2019, 1, 25))
 
-        travel_to Time.zone.local(2019, 1, 25)
-        assert_equal(0, model.weeks_ago)
+        travel_to Time.zone.local(2019, 1, 25) do
+          assert_equal(0, model.weeks_ago)
+        end
+
+        travel_to Time.zone.local(2019, 1, 27) do
+          assert_equal(0, model.weeks_ago)
+        end
+
+        travel_to Time.zone.local(2019, 2, 8) do
+          assert_equal(2, model.weeks_ago)
+        end
 
-        travel_to Time.zone.local(2019, 1, 27)
-        assert_equal(0, model.weeks_ago)
+        travel_to Time.zone.local(2019, 2, 9) do
+          assert_equal(2, model.weeks_ago)
+        end
 
-        travel_to Time.zone.local(2019, 2, 8)
-        assert_equal(2, model.weeks_ago)
+        travel_to Time.zone.local(2019, 2, 11) do
+          assert_equal(3, model.weeks_ago)
+        end
 
-        travel_to Time.zone.local(2019, 2, 9)
-        assert_equal(2, model.weeks_ago)
+        # Regression: DST boundaries should not affect week math.
+        Time.use_zone('Eastern Time (US & Canada)') do
+          dst_model = create_product_by_week(reporting_on: Time.zone.local(2019, 3, 4, 0, 0, 0))
 
-        travel_to Time.zone.local(2019, 2, 11)
-        assert_equal(3, model.weeks_ago)
+          travel_to Time.zone.local(2019, 3, 11, 0, 0, 0) do
+            assert_equal(1, dst_model.weeks_ago)
+          end
+        end
       end
     end
   end

testing/lib/workarea/test_case.rb

@@ -71,10 +71,23 @@ module Workers
     module SearchIndexing
       extend ActiveSupport::Concern
 
+      def wait_for_elasticsearch!(timeout: 30)
+        start = Time.now
+
+        loop do
+          Workarea.elasticsearch.cluster.health(wait_for_status: "yellow")
+          break
+        rescue Faraday::ConnectionFailed, ::Elasticsearch::Transport::Transport::Errors::ServiceUnavailable, ::Elasticsearch::Transport::Transport::Errors::BadGateway
+          raise if (Time.now - start) > timeout
+          sleep 0.5
+        end
+      end
+
       included do
         setup do
           Workarea.config.auto_refresh_search = true
           WebMock.disable_net_connect!(allow_localhost: true)
+          wait_for_elasticsearch!
           Workarea::Elasticsearch::Document.all.each(&:reset_indexes!)
           Workarea::Search::Storefront.ensure_dynamic_mappings
         end
@@ -241,9 +254,21 @@ module Setup
       extend ActiveSupport::Concern
       include ActiveJob::TestHelper
 
+      def truncate_all_mongoid_clients!
+        # Mongoid.truncate! only truncates the global (default) client.
+        # Workarea uses additional clients (e.g. :metrics), so ensure we clear
+        # data for all configured clients to avoid cross-test pollution.
+        Mongoid::Clients.clients.values.each do |client|
+          client.database.collections.each do |collection|
+            next if collection.name.start_with?('system.')
+            collection.find.delete_many
+          end
+        end
+      end
+
       included do
         setup do
-          Mongoid.truncate!
+          truncate_all_mongoid_clients!
           Workarea.redis.flushdb
           WebMock.disable_net_connect!(allow_localhost: true)
           ActionMailer::Base.deliveries.clear