[COMMS-938] In numeric mode, search semantic identifiers only when hash-prefixed

https://community.openproject.org/wp/COMMS-938

This has two advantages:
1. It's easier to understand since it works the same way in classic
   identifier mode: Only when the number is prefixed with a hash the ID
   is searched
2. Otherwise in classic identifier mode the WorkPackageSemanticAlias
   table would be hit for every string that starts with a letter, which
   is unnecessary and makes the search even slower

Author: judithroth

app/models/queries/work_packages/filter/typeahead_filter.rb

@@ -44,21 +44,21 @@ def where
     parts.map { |part| "(#{conditions_for(part).join(' OR ')})" }.join(" AND ")
   end
 
-  def conditions_for(part)
+  def conditions_for(part) # rubocop:disable Metrics/AbcSize
     conditions = [subject_condition(part),
                   project_name_condition(part),
                   type_name_condition(part),
                   status_condition(part)]
 
     if (match = part.match(/\A(#)?(#{Projects::Identifier::SEMANTIC_FORMAT.source}-?\d*)\z/i))
-      conditions << work_package_identifier_condition(match[2])
+      conditions << work_package_identifier_condition(hash_prefixed: match[1].present?, search_term: match[2])
     end
 
     if (match = part.match(/\A(#)?(\d+)\z/))
       conditions << id_or_sequence_number_condition(hash_prefixed: match[1].present?, search_term: match[2])
     end
 
-    conditions
+    conditions.compact
   end
 
   def subject_condition(string)
@@ -69,14 +69,6 @@ def project_name_condition(string)
     Queries::Operators::Contains.sql_for_field([string], Project.table_name, "name")
   end
 
-  def work_package_identifier_condition(string)
-    alias_condition = Queries::Operators::StartsWith.sql_for_field(
-      [string], WorkPackageSemanticAlias.table_name, "identifier"
-    )
-    "#{WorkPackage.table_name}.id IN " \
-      "(SELECT work_package_id FROM #{WorkPackageSemanticAlias.table_name} WHERE #{alias_condition})"
-  end
-
   def type_name_condition(string)
     Queries::Operators::Contains.sql_for_field([string], Type.table_name, "name")
   end
@@ -99,6 +91,16 @@ def status_condition(string)
     end
   end
 
+  def work_package_identifier_condition(hash_prefixed:, search_term:)
+    return unless Setting::WorkPackageIdentifier.semantic? || hash_prefixed
+
+    alias_condition = Queries::Operators::StartsWith.sql_for_field(
+      [search_term], WorkPackageSemanticAlias.table_name, "identifier"
+    )
+    "#{WorkPackage.table_name}.id IN " \
+      "(SELECT work_package_id FROM #{WorkPackageSemanticAlias.table_name} WHERE #{alias_condition})"
+  end
+
   def id_or_sequence_number_condition(hash_prefixed:, search_term:)
     if Setting::WorkPackageIdentifier.classic? || hash_prefixed
       id_condition(search_term)

app/models/queries/work_packages/selects/exact_match_select.rb

@@ -65,18 +65,13 @@ def self.exact_match_condition_sql(query_string)
     stripped = query_string.to_s.strip
     return nil if stripped.blank? || stripped.match?(/\s/)
 
+    hash_prefixed = stripped.start_with?("#")
     candidate = stripped.delete_prefix("#")
     condition =
       if candidate.match?(/\A[1-9]\d*\z/)
-        numeric_exact_match_condition(candidate, hash_prefixed: stripped.start_with?("#"))
+        numeric_exact_match_condition(candidate, hash_prefixed:)
       elsif candidate.match?(/\A#{WorkPackage::SemanticIdentifier::SEMANTIC_ID_PATTERN.source}\z/i)
-        # So far, semantic identifiers are always upper case.
-        # We can leverage this to match in a way that allows index usage.
-        OpenProject::SqlSanitization.sanitize(
-          "#{WorkPackage.table_name}.id IN (SELECT work_package_id FROM " \
-          "#{WorkPackageSemanticAlias.table_name} WHERE identifier = ?)",
-          candidate.upcase
-        )
+        semantic_exact_match_condition(candidate, hash_prefixed:)
       end
 
     return nil unless condition
@@ -95,4 +90,16 @@ def self.numeric_exact_match_condition(candidate, hash_prefixed:)
       )
     end
   end
+
+  def self.semantic_exact_match_condition(candidate, hash_prefixed:)
+    return nil unless Setting::WorkPackageIdentifier.semantic? || hash_prefixed
+
+    # So far, semantic identifiers are always upper case.
+    # We can leverage this to match in a way that allows index usage.
+    OpenProject::SqlSanitization.sanitize(
+      "#{WorkPackage.table_name}.id IN (SELECT work_package_id FROM " \
+      "#{WorkPackageSemanticAlias.table_name} WHERE identifier = ?)",
+      candidate.upcase
+    )
+  end
 end

spec/models/queries/work_packages/filter/typeahead_filter_spec.rb

@@ -352,14 +352,23 @@
                identifier: "PHO-2")
       end
 
-      context "and there are still entries in the semantic alias registry" do
-        let(:values) { [identifier_work_package1_semantic_alias.identifier] }
+      context "when searching by a semantic identifier prefixed with '#'" do
+        let(:values) { ["##{identifier_work_package1_semantic_alias.identifier}"] }
 
         it "still finds by existing identifiers" do
           expect(subject).to include(identifier_work_package1)
           expect(subject).not_to include(identifier_work_package2)
         end
       end
+
+      context "when searching by a semantic identifier not prefixed with '#'" do
+        let(:values) { [identifier_work_package1_semantic_alias.identifier] }
+
+        it "does not find by existing identifiers" do
+          expect(subject).not_to include(identifier_work_package1)
+          expect(subject).not_to include(identifier_work_package2)
+        end
+      end
     end
 
     context "when searching by status" do

spec/models/queries/work_packages/selects/exact_match_select_spec.rb

@@ -121,34 +121,54 @@ def ranked_ids(ids)
       end
       let(:query_string) { "COM-5" }
 
-      it "ranks the exact identifier match above one that only shares the prefix" do
-        expect(ranked_ids([exact_work_package.id, prefix_work_package.id]).first)
-          .to eq(exact_work_package.id)
-      end
-
-      context "when the query is given in lower case" do
-        let(:query_string) { "com-5" }
-
-        it "still matches COM-5" do
+      context "and the instance is in semantic identifier mode",
+              with_settings: { work_packages_identifier: Setting::WorkPackageIdentifier::SEMANTIC } do
+        it "ranks the exact identifier match above one that only shares the prefix" do
           expect(ranked_ids([exact_work_package.id, prefix_work_package.id]).first)
             .to eq(exact_work_package.id)
         end
-      end
 
-      context "when matching a historical alias in classic mode",
-              with_settings: { work_packages_identifier: Setting::WorkPackageIdentifier::CLASSIC } do
-        it "still matches through the alias table" do
-          expect(ranked_ids([exact_work_package.id, prefix_work_package.id]).first)
-            .to eq(exact_work_package.id)
+        context "when the query is given in lower case" do
+          let(:query_string) { "com-5" }
+
+          it "still matches COM-5" do
+            expect(ranked_ids([exact_work_package.id, prefix_work_package.id]).first)
+              .to eq(exact_work_package.id)
+          end
+        end
+
+        # context "when matching a historical alias in classic mode",
+        #         with_settings: { work_packages_identifier: Setting::WorkPackageIdentifier::CLASSIC } do
+        #   it "still matches through the alias table" do
+        #     expect(ranked_ids([exact_work_package.id, prefix_work_package.id]).first)
+        #       .to eq(exact_work_package.id)
+        #   end
+        # end
+
+        context "when the query is hash-prefixed" do
+          let(:query_string) { "#COM-5" }
+
+          it "still ranks the exact identifier match above one that only shares the prefix" do
+            expect(ranked_ids([exact_work_package.id, prefix_work_package.id]).first)
+              .to eq(exact_work_package.id)
+          end
         end
       end
 
-      context "when the query is hash-prefixed" do
-        let(:query_string) { "#COM-5" }
+      context "and the instance is in classic identifier mode" do
+        context "when the query is hash-prefixed" do
+          let(:query_string) { "#COM-5" }
 
-        it "still ranks the exact identifier match above one that only shares the prefix" do
-          expect(ranked_ids([exact_work_package.id, prefix_work_package.id]).first)
-            .to eq(exact_work_package.id)
+          it "still ranks the exact identifier match above one that only shares the prefix" do
+            expect(ranked_ids([exact_work_package.id, prefix_work_package.id]).first)
+              .to eq(exact_work_package.id)
+          end
+        end
+
+        context "when the query is not hash-prefixed" do
+          let(:query_string) { "COM-5" }
+
+          it { is_expected.to be_nil }
         end
       end
     end

spec/models/query/sort_criteria_spec.rb

@@ -97,7 +97,7 @@
       context "when a typeahead filter is active on the query" do
         let(:query) do
           build_stubbed(:query, show_hierarchies: false).tap do |q|
-            q.add_filter(:typeahead, "**", "COM-5")
+            q.add_filter(:typeahead, "**", "#COM-5")
           end
         end
 

spec/requests/api/v3/work_packages/available_relation_candidates_resource_spec.rb

@@ -138,7 +138,7 @@ def work_packages
 
     describe "relation candidates for wp1 (in hierarchy) with exact_match sorting" do
       let(:href) do
-        "/api/v3/work_packages/#{wp1.id}/available_relation_candidates?query=RELCAND-5" \
+        "/api/v3/work_packages/#{wp1.id}/available_relation_candidates?query=%23RELCAND-5" \
           "&sortBy=[[\"exactMatch\",\"desc\"],[\"updatedAt\",\"desc\"]]"
       end
 

spec/requests/api/v3/work_packages/index_resource_spec.rb

@@ -128,7 +128,7 @@
           {
             typeahead: {
               operator: "**",
-              values: "COM-5"
+              values: "#COM-5"
             }
           }
         ]