FEATURE: autocomplete usernames early in topic based on participation

FEATURE: autocomplete usernames early in topic based on participation

Following this change when a user hits @ and is replying to a topic they will see usernames of people who were last seen and participated in the topic

This is somewhat experimental, we may tweak this, or make it optional.

Also, a regression in a423a938 where hitting TAB would eat a post you were writing:

Eg this would eat a post:

@hello, testing 123 <tab>

diff --git a/app/assets/javascripts/discourse/lib/user-search.js.es6 b/app/assets/javascripts/discourse/lib/user-search.js.es6
index 932d93b..c64ec38 100644
--- a/app/assets/javascripts/discourse/lib/user-search.js.es6
+++ b/app/assets/javascripts/discourse/lib/user-search.js.es6
@@ -23,7 +23,11 @@ function performSearch(
     resultsFn(cached);
     return;
   }
-  if (term === "") {
+
+  // I am not strongly against unconditionally returning
+  // however this allows us to return a list of probable
+  // users we want to mention, early on a topic
+  if (term === "" && !topicId) {
     return [];
   }
 
@@ -108,6 +112,18 @@ function organizeResults(r, options) {
   return results;
 }
 
+// all punctuations except for . which is allowed in usernames
+// note: these are valid in names, but will end up tripping search anyway so just skip
+// this means searching for `sam saffron` is OK but if my name is `sam$ saffron` autocomplete
+// will not find me, which is a reasonable compromise
+//
+// we also ignore if we notice a double space or a string that is only a space
+const ignoreRegex = /([\u2000-\u206F\u2E00-\u2E7F\\'!"#$%&()*+,\-\/:;<=>?@\[\]^_`{|}~])|\s\s|^\s$/;
+
+function skipSearch(term) {
+  return !!term.match(ignoreRegex);
+}
+
 export default function userSearch(options) {
   if (options.term && options.term.length > 0 && options.term[0] === "@") {
     options.term = options.term.substring(1);
@@ -121,10 +137,6 @@ export default function userSearch(options) {
     topicId = options.topicId,
     group = options.group;
 
-  if (/[^\w.-]/.test(term)) {
-    term = "";
-  }
-
   if (oldSearch) {
     oldSearch.abort();
     oldSearch = null;
@@ -143,6 +155,11 @@ export default function userSearch(options) {
       resolve(CANCELLED_STATUS);
     }, 5000);
 
+    if (skipSearch(term)) {
+      resolve([]);
+      return;
+    }
+
     debouncedSearch(
       term,
       topicId,
diff --git a/app/models/user_search.rb b/app/models/user_search.rb
index 3acdf91..98855b5 100644
--- a/app/models/user_search.rb
+++ b/app/models/user_search.rb
@@ -80,7 +80,13 @@ class UserSearch
 
     # 2. in topic
     if @topic_id
-      filtered_by_term_users.where('users.id IN (SELECT p.user_id FROM posts p WHERE topic_id = ?)', @topic_id)
+      in_topic = filtered_by_term_users.where('users.id IN (SELECT p.user_id FROM posts p WHERE topic_id = ?)', @topic_id)
+
+      if @searching_user.present?
+        in_topic = in_topic.where('users.id <> ?', @searching_user.id)
+      end
+
+      in_topic
         .order('last_seen_at DESC')
         .limit(@limit - users.length)
         .pluck(:id)
@@ -90,10 +96,12 @@ class UserSearch
     return users.to_a if users.length >= @limit
 
     # 3. global matches
-    filtered_by_term_users.order('last_seen_at DESC')
-      .limit(@limit - users.length)
-      .pluck(:id)
-      .each { |id| users << id }
+    if !@topic_id || @term.present?
+      filtered_by_term_users.order('last_seen_at DESC')
+        .limit(@limit - users.length)
+        .pluck(:id)
+        .each { |id| users << id }
+    end
 
     users.to_a
   end
diff --git a/spec/models/user_search_spec.rb b/spec/models/user_search_spec.rb
index 73b8588..2f4b9c4 100644
--- a/spec/models/user_search_spec.rb
+++ b/spec/models/user_search_spec.rb
@@ -137,6 +137,11 @@ describe UserSearch do
 
     results = search_for(staged.username, include_staged_users: true)
     expect(results.first.username).to eq(staged.username)
+
+    results = search_for("", topic_id: topic.id, searching_user: user1)
+
+    # mrb is omitted, mrb is current user
+    expect(results.map(&:username)).to eq(["mrpink", "mrorange"])
   end
 
 end
diff --git a/test/javascripts/lib/user-search-test.js.es6 b/test/javascripts/lib/user-search-test.js.es6
index 647a0ab..55b512c 100644
--- a/test/javascripts/lib/user-search-test.js.es6
+++ b/test/javascripts/lib/user-search-test.js.es6
@@ -1,5 +1,4 @@
 import userSearch from "discourse/lib/user-search";
-import { CANCELLED_STATUS } from "discourse/lib/autocomplete";
 
 QUnit.module("lib:user-search", {
   beforeEach() {
@@ -73,7 +72,29 @@ QUnit.test("it strips @ from the beginning", async assert => {
   assert.equal(results[results.length - 1]["name"], "team");
 });
 
-QUnit.test("it does not search for invalid usernames", async assert => {
-  let results = await userSearch({ term: "foo, " });
-  assert.equal(results, CANCELLED_STATUS);
+QUnit.test("it skips a search depending on punctuations", async assert => {
+  let skippedTerms = [
+    "@sam  s", // double space is not allowed
+    "@sam;",
+    "@sam,",
+    "@sam:"
+  ];
+
+  skippedTerms.forEach(async term => {
+    let results = await userSearch({ term });
+    assert.equal(results.length, 0);
+  });
+
+  let allowedTerms = [
+    "@sam sam", // double space is not allowed
+    "@sam.sam",
+    "@"
+  ];
+
+  let topicId = 100;
+
+  allowedTerms.forEach(async term => {
+    let results = await userSearch({ term, topicId });
+    assert.equal(results.length, 6);
+  });
 });

GitHub sha: 1f4ace4f

3 Likes

FEATURE: add missing autocomplete suppressions

REFACTOR: remove disallowEmails option from user-selector